by

Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Jul 12 18:39

    jgaskins on experimental-bolt-conversion

    Fix tiny negative ints How did… (compare)

  • Jul 03 22:07

    jgaskins on experimental-bolt-conversion

    Remove retries from the connect… Use lock-free atomic mutation i… Increase max idle pool size Th… and 1 more (compare)

  • Jun 05 02:56

    jgaskins on experimental-bolt-conversion

    Handle busted exec_cast queries… (compare)

  • Jun 03 03:01

    jgaskins on experimental-bolt-conversion

    Clarify specs, add Bolt seriali… (compare)

  • May 06 23:51

    jgaskins on experimental-bolt-conversion

    Make Driver a superclass of bot… (compare)

  • Apr 29 23:06

    jgaskins on experimental-bolt-conversion

    Add experimental to_bolt_params… (compare)

  • Apr 17 21:48
    jgaskins closed #7
  • Apr 17 01:01
    anamba commented #7
  • Apr 16 23:15
    jgaskins commented #7
  • Apr 16 23:13

    jgaskins on master

    Update README with cluster supp… (compare)

  • Apr 16 22:52

    jgaskins on v0.6.0

    (compare)

  • Apr 16 22:52

    jgaskins on master

    Version 0.6.0 (compare)

  • Apr 16 22:51

    jgaskins on master

    Add initial support for clusters Add SNI for SSL connection Merge branch 'master' into add-… and 20 more (compare)

  • Apr 16 22:48

    jgaskins on add-cluster-support

    Update Cluster for Crystal 0.34 Don't spin up an initial connec… (compare)

  • Apr 14 04:14

    jgaskins on add-cluster-support

    Fix StreamingResultSet Support Crystal 0.34 (compare)

  • Apr 08 12:47
    jgaskins commented #7
  • Apr 08 08:28
    anamba commented #7
  • Apr 08 08:26
    anamba opened #7
  • Mar 20 01:30

    jgaskins on add-cluster-support

    Fix compatibility with Crystal … Enforce string key in failure r… (compare)

  • Feb 27 01:31

    jgaskins on add-cluster-support

    Make cluster refresh more resil… (compare)

Aaron Namba
@anamba
for now i will probably just work around it by not doing db stuff inside spawn but man this is weird
oh but wait, maybe it is your dept :)
just found that the thing that's infinitely recursing is here: from /home/app/myapp/lib/neo4j/src/neo4j/bolt/connection.cr:189:9 in 'run'
Jamie Gaskins
@jgaskins
hmmmm.... that does sound weird. the driver doesn't raise IO::Timeout explicitly, but maybe something happens under the hood that does. Do you have a small test case for it?
Aaron Namba
@anamba
i don't unfortunately... i haven't been able to figure out where it's coming from until just now
Jamie Gaskins
@jgaskins
okay, i'll give that a shot on the plane, too.
oh wait
if the whole pool.connection block isn't within the spawn block, i could imagine that happening
Aaron Namba
@anamba
ohhhhhh yeah i think i see why it's upset
Jamie Gaskins
@jgaskins
right now the whole thing is synchronous and we need to pull all the data out before checking the connection back in.
the whole execute method, that is
Aaron Namba
@anamba
i'm finding the object in one fiber and doing work on it in another
that might not be the best idea
Jamie Gaskins
@jgaskins
hmmm, yeah, maybe a Channel might be useful there?
Aaron Namba
@anamba
i didn't do that in ruby, not sure why i decided to do it that way when porting to crystal
well it is using a channel. but passing the entire object. i'll pass an id instead like i was doing with ruby threads
Jamie Gaskins
@jgaskins
fibers being closures make that a very tempting thing to do :-)
ahhh, okay
i think the fact that the driver doesn't provide its own connection pool probably opens it up to things like this. if the driver managed the connection pool itself (like the Java one does), it might avoid this kind of situation.
i'm beginning to see why the Java driver has so much ceremony surrounding it
managing these connections is kinda wild
and it's probably the only way we'll be able to do lazy consumption like i really want to do :laughing:
that is, execute would return an Iterable(Array(Neo4j::Type))) instead of an Enumerable(Array(Neo4j::Type))
so you could begin consuming results before you receive the whole thing
Aaron Namba
@anamba
i'd really like that, it would make my neo4j_model ORM more efficient
Jamie Gaskins
@jgaskins
yeah, definitely
lower latency to first result and possibly reduced memory usage for larger queries
Aaron Namba
@anamba
yep right now my memory usage is a bit higher than i'd like. not ActiveRecord high, but still
Jamie Gaskins
@jgaskins
yeah, the entire response string and its deserialized form in memory at the same time is not ideal.
Aaron Namba
@anamba
i still haven't been able to figure out what is triggering the infinite recursion (and relately, also haven't figured out why you are doing that :smile:)
Aaron Namba
@anamba
well, still not entirely sure what the root issue is, but i got my app back online at least by monkey patching your thing to kill the infinite recursion:
module Neo4j
  module Bolt
    class Connection
      private def run(statement, parameters = {} of String => Type, retries = 0)
        write_message do |msg|
          msg.write_structure_start 2
          msg.write_byte Commands::Run
          msg.write statement
          msg.write parameters
        end

        result = read_result
        case result
        when Failure
          raise ::Neo4j::QueryException.new(result.attrs["message"].as(String), result.attrs["code"].as(String))
        when Success, Ignored
          result
        else
          raise ::Neo4j::UnknownResult.new("Cannot identify this result: #{result.inspect}")
        end
      rescue ex : IO::EOFError
        initialize @uri, @ssl
        if retries < 10
          sleep 0.1
          run statement, parameters, retries + 1
        else
          raise ex
        end
      end
    end
  end
end
Jamie Gaskins
@jgaskins
Oh, that was to self-heal connections when they get disconnected.
So it was only expected to be called once. Do you know what caused it to keep running recursively?
Jamie Gaskins
@jgaskins
@anamba Also I found a way last night to stream data from the DB using a Channel(Array(Type)). It's super naive right now and requires you to completely consume results before sending any followup queries (so, before a transaction block ends, for example), but it reduces latency to first row and kept my example query (millions of nodes) at 1.4MB RAM compared to 2.4GB when the query result is consumed eagerly.
It was also surprisingly faster, probably due to being able to reuse memory in the GC. I don't know if I've pushed the branch up yet but if not I'll push it later today when I get to a computer.
Jamie Gaskins
@jgaskins
@anamba Here's my initial solution for streaming: https://github.com/jgaskins/neo4j.cr/compare/streaming-results
It's really naive right now but it does work from what I can tell as long as you consume all of the results before sending off any further queries.
Aaron Namba
@anamba
But if you don't, the penalty is blocking forever, right? :smile:
Jamie Gaskins
@jgaskins
If you have to break processing, connection.reset should bring it back to a fresh state. I've had a few attempts at streaming responses and pipelining queries. So far, this has been my most successful. Or my least unsuccessful. :-)
Aaron Namba
@anamba
@jgaskins okay, so... i think you're gonna love this one. i think i figured out how to trigger that infinite recursion bug
i finally found a database record that would trigger the error every time. i have a field i was trying to read, increment and save back. it's supposed to be positive integer, but for some reason the value was -32. trying to increment to -31 and save triggers the infinite recursion.
i tried a few more values just for fun. -33 is fine. -32, -31, -30, -20 are bad. -16 is fine. -17 is bad. what on earth dude
Aaron Namba
@anamba
so... for now i'm gonna avoid those values (they weren't supposed to be negative anyway). it looks like the order in which i add things when incrementing makes a huge difference, due to the behavior of neo4j.cr (returns the smallest int type that can fit the result). 127_i8+1 = -127_i8, but 1+127_i8 = 128_i32 arghhhhh
Aaron Namba
@anamba
:joy: :laughing: just deployed. working great
Jamie Gaskins
@jgaskins
Oooh, nice catch. I remember looking at this method and wondering if I should re-write it as multiple overloaded methods instead of that big blob. I want to say that code came from the MsgPack shard (which was probably intended to streamline payloads), but yeah, if it's causing errors, I'll go ahead and rewrite it.
Jamie Gaskins
@jgaskins
Yep, looks like that was from my copy pasta from the msgpack shard
Jamie Gaskins
@jgaskins
Now that I think about it, I think Neo4j might actually send us the numbers in those sizes for the same reason MessagePack shrinks the type, so it may be about how we read it. I'll double-check this real quick.
Because each of these specs pass:
      {
        0_i8,
        -1_i8,
        1_i8,
        Int8::MAX,
        Int8::MIN,

        0_i16,
        -1_i16,
        1_i16,
        Int8::MIN.to_i16 - 1,
        Int8::MAX.to_i16 + 1,
        Int16::MIN,
        Int16::MAX,

        0_i32,
        -1_i32,
        1_i32,
        Int16::MIN.to_i32 - 1,
        Int16::MAX.to_i32 + 1,
        Int32::MIN,
        Int32::MAX,

        0__i64,
        -1_i64,
        1__i64,
        Int32::MIN.to_i64 - 1,
        Int32::MAX.to_i64 + 1,
        Int64::MIN,
        Int64::MAX,
      }.each do |int|
        it "serializes #{int}" do
          unpack(pack(int)).should eq int
        end
      end
Unless I've missed something
Either way, I don't know what the implications of just boosting all ints to Int64 but that certainly might help with your overflows.