Skip to content

Conversation

@lutovich
Copy link
Contributor

This PR makes driver close the network channel when either RESET or ACK_FAILURE receive an error response. Message RESET is sent before release of the channel back to the channel pool. It might fail if database experiences a fatal error (like OutOfMemoryError) or a network failure occurs. In case of such errors channel will be closed and returned to the pool, which will drop it later. Message ACK_FAILURE is sent to acknowledge a FAILURE response. Failure to do so is unrecoverable and should result in a connection being dropped.

@lutovich lutovich requested a review from ali-ince May 21, 2018 21:28
@lutovich lutovich force-pushed the 1.7-failure-on-reset-and-ack-failure branch from 3849411 to f9efc0b Compare May 24, 2018 18:09
Copy link
Contributor

@ali-ince ali-ince left a comment

Choose a reason for hiding this comment

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

LGTM. I made one comment, which was not directly obvious to me.

}

protected void resetCompleted( CompletableFuture<Void> completionFuture )
protected void resetCompleted( CompletableFuture<Void> completionFuture, boolean success )
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we loose information without using the RESET outcome, i.e. success parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This response handler is used only for Session#reset() which is a best-effort call to clear the connection state on the database side. It might succeed, fail or do nothing regardless of a network error. That is why I thought it is okay to simply notify about the completion of the remote call here. Session#reset() is also deprecated :)

Does this make sense?

This commit makes driver close the network channel when either `RESET`
or `ACK_FAILURE` receive an error response. Message `RESET` is sent
before release of the channel back to the channel pool. It might fail
if database experiences a fatal error (like `OutOfMemoryError`) or a
network failure occurs. In case of such errors channel will be closed
and returned to the pool, which will drop it later. Message
`ACK_FAILURE` is sent to acknowledge a `FAILURE` response. Failure
to do so is unrecoverable and should result in a connection
being dropped.
@lutovich lutovich force-pushed the 1.7-failure-on-reset-and-ack-failure branch from f9efc0b to ce57a1d Compare May 29, 2018 08:17
@ali-ince ali-ince merged commit 6cba6b0 into neo4j:1.7 May 29, 2018
@lutovich lutovich deleted the 1.7-failure-on-reset-and-ack-failure branch May 29, 2018 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants