by

Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • 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)

  • Jan 31 04:09

    jgaskins on add-cluster-support

    Add neo4j as a cluster URL sche… (compare)

  • Jan 23 23:31
    jgaskins commented #6
Jamie Gaskins
@jgaskins
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.
Wait, didn't Crystal add overflow detection in 0.27?
Jamie Gaskins
@jgaskins

In this version, a couple of arithmetic operators were added thanks to #6890: &+, &-, &. They will perform additions, subtraction and multiplication with wrapping (as in, not overflowing). Some may notice that that is the exact same behaviour as +, -, . In a future version, the regular operators are going to raise on overflow. This will allow you to trust the result of the operations when reaching the limits of the representable range. The ampersand operators might be useful to think of them as applying an & 0xFF bit mask to the result.

Ah, looks like they only aliased the current, non-raising versions so people can migrate their code if they don't want to raise on overflow. https://crystal-lang.org/2018/11/01/crystal-0.27.0-released.html

It was stack overflow detection they added for real.
Jamie Gaskins
@jgaskins
Ugh, looks like Crystal 0.27.2 broke my async_it monkeypatch on Spec … or I'll have to come up with a different approach to run integration specs concurrently. :joy:
Jamie Gaskins
@jgaskins

Hmm, so it looks like Neo4j is sending us the numbers in the smallest type possible:

result = connection.execute "RETURN $int8, $int16, $int32, $int64",
  int8: Int8::MAX,
  int16: Int16::MAX,
  int32: Int32::MAX,
  int64: Int64::MAX

pp result.first
# => [127_i8, 32767_i16, 2147483647, 9223372036854775807_i64]

We're deserializing it in whatever integer type Neo4j tells us it is. :-\

@anamba I kinda wanna just upcast everything to Int64 to avoid the int-overflow issue you're seeing, but also I think that might break things like Neo4j.map_node(some_int_value: Int16).
Not sure of a way to navigate this yet.
Do you have any thoughts?
Aaron Namba
@anamba
eh, i think it's ok. it's more of a crystal gotcha than a neo4j.cr gotcha i think
but the issue of not being able to save an int value in the range of -17 to -32 is pretty strange
Jamie Gaskins
@jgaskins
Okay, cool. I'll still give it a look and see what the implications are. I normally use map_node and map_relationship to extract nodes and rels to structs and use them as value objects.
Yeah, that part is weird. I wonder if that's also an issue in MsgPack
Lemme run a quick test.
Jamie Gaskins
@jgaskins
Oooh, it looks like it was meant to save a byte if it could. If it's in a certain range, the protocol can cram the byte marker and the value into the same byte, but I didn't decode it properly. I'm just gonna go ahead and encode the byte marker and the value in separate bytes for now until I understand how it works.
@anamba Fixed and pushed v0.2.3
Aaron Namba
@anamba
:confetti_ball:
Jamie Gaskins
@jgaskins
Sorry about that, man
Aaron Namba
@anamba
you have no idea :joy:
that was a WEIRD one
this binary protocol stuff makes for some pretty strange corner cases. not in the corners i usually look for