Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Catching up with TestKit #520

Merged
merged 19 commits into from
Aug 18, 2023
Merged

Catching up with TestKit #520

merged 19 commits into from
Aug 18, 2023

Conversation

robsdedude
Copy link
Member

@robsdedude robsdedude commented Aug 8, 2023 •
edited
Loading

  1. Add support for routing table specific TestKit messaged (retrieving routing tables and forcing updates)
  2. Improve logging in the TestKit backend (all logs-driver and bolt-got both to stdout and TestKit) for easier debugging
  3. Overhaul how/when the driver drops servers from the cached routing table:
    • Move logic of deactivating servers on failures into the pool. This functionality should not only be present when using transaction functions.
    • Drop writers on certain error codes: Neo.ClientError.Cluster.NotALeader and Neo.ClientError.General.ForbiddenOnReadOnlyDatabase
  4. To simplify the code and avoid deadlocks or inconsistent driver states, both pool and router use blocking locks now. However, they will never perform IO while holding the lock removing the need for lock acquisition timeouts.

Depends on:

@robsdedude
Reformat feature list to reflect TestKit's state
f65467d
@robsdedude
Clean up test skips
23d5cb8
@robsdedude
Go driver does not support liveness check
63e9d09
@robsdedude
TestKit backend: improve logging
bd45324 
Log everything to stdout + TestKit socket.
@robsdedude
Drop servers from routing table on IO failure
8336f50
@robsdedude
Remove writer from routing table on certain errors
509675f
@robsdedude
Refactor responsibilities and locking
17cbfc6 
The retry state is no longer responsible for invalidating servers on broken
connections. The pool and/or routing logic should take care of that. This is
necessary and logical since invalidation should happen regardless of which API
(session.Run, transaction.Run, ...) is used.

Pool and router no longer do any IO while holding locks. Hence, the locks can be
turned into blocking locks without risking blocking for too long.
@robsdedude
Add support for RT related TestKit messages
1befcaa
@robsdedude
Merge branch '5.0' into testkit-update-backend
695cb28
@robsdedude robsdedude mentioned this pull request Aug 8, 2023
Closed
fbiville and others added 2 commits August 9, 2023 14:46
@fbiville @robsdedude
Remove unused context parameters
caf4b38 
Signed-off-by: Rouven Bauer <rouven.bauer@neo4j.com>
@robsdedude
Remove more unused ctx parameters
c4a241f
StephenCathcart
StephenCathcart approved these changes Aug 16, 2023
View reviewed changes
Copy link
Contributor

@StephenCathcart StephenCathcart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! :shipit:

fbiville
fbiville approved these changes Aug 17, 2023
View reviewed changes
robsdedude and others added 6 commits August 17, 2023 16:08
@robsdedude
Fix not removing server from all routing tables
a7ed663 
Also removing error return value from all router.Invalidate* calls
@robsdedude
TestKit backend: routing table response
14e2afb 
TestKit expects the default database to be `null` not an empty string.
So we need to translate.
@robsdedude
Add (back) test skips
1ab56cd
@fbiville @robsdedude
Rename SelectedDatabase() -> Database()
fe9a765 
Signed-off-by: Rouven Bauer <rouven.bauer@neo4j.com>
@fbiville @robsdedude
Remove error return value from internal infallible interface functions
aa0db6c 
Signed-off-by: Rouven Bauer <rouven.bauer@neo4j.com>
@robsdedude
Fix silly copy paste error
5ecc3c6
@robsdedude robsdedude merged commit 39d4411 into neo4j:5.0 Aug 18, 2023
@robsdedude robsdedude deleted the testkit-update-backend branch August 18, 2023 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbiville fbiville fbiville approved these changes

@StephenCathcart StephenCathcart StephenCathcart approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@robsdedude @fbiville @StephenCathcart