-
Notifications
You must be signed in to change notification settings - Fork 67
Add bookmarks support #238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I see tests fail since they run without the |
You can run the test only when the feature is enabled, e.g. neo4rs/lib/tests/result_summary.rs Line 1 in afbe45d
|
I fixed the issue, but I had to run the new integration test under the feature |
No, it's fine. I think now I overdid it with the various unstable subfeatures, they should probably collapsed into the unstable-v1 feature instead of new ones being added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this looks good.
One change it would like to make is to not have a signature change on the commit
method when the unstable feature is not enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 👍
Thanks for the PR, this looks great! |
This PR aims to add support for bookmarks in transactions. It also collects bookmarks used in a routed manager and sends them when requesting a new routing table. The list of collected bookmarks is cleared up every time a READ operation returns no bookmarks (that should mean the state of the cluster is consistent).
The bookmarks are sent in the BEGIN message and retrieved from the COMMIT message.
Doubts
I read somewhere that the command HELLO can also contain bookmarks, but the documentation does not say a thing about it.
Bonus: While implementing this feature, I found a problem when getting a connection in a routed environment, caused by a synchronization problem between the manager and the thread fetching the routing table.
Changes
Transaction Handling Updates:
Begin
request type with associated metadata and builder pattern for constructing instances. (lib/src/bolt/request/begin.rs
)Commit
request type to include aCommitResponse
that handles bookmarks. (lib/src/bolt/request/commit.rs
) [1] [2] [3]Graph
struct to support starting transactions with bookmarks and to handle bookmarks in query execution results. (lib/src/graph.rs
) [1] [2] [3] [4] [5] [6]Routing Mechanism Updates:
Begin
request type to thebolt/request/mod.rs
module. (lib/src/bolt/request/mod.rs
) [1] [2]Route
request type to remove lifetime parameters and updated serialization logic. (lib/src/bolt/request/route.rs
) [1] [2] [3] [4] [5] [6]Miscellaneous:
Bookmark
trait to standardize bookmark handling across different response types. (lib/src/bookmarks.rs
)Connection
struct to support the newRoute
request type without lifetime parameters. (lib/src/connection.rs
)