-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
couchbase: wait until query engine knows about bucket before creating… #2662
Conversation
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.
This is a good idea - thanks. Just spotted a possible resource leak; once that's resolved I'd be happy to merge.
// If the query service is enabled, make sure that we only proceed if the query engine also | ||
// knows about the bucket in its metadata configuration. | ||
Unreliables.retryUntilTrue(1, TimeUnit.MINUTES, () -> { | ||
Response queryResponse = doHttpRequest(QUERY_PORT, "/query/service", "POST", new FormBody.Builder() |
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.
Just a thought: all these Response
objects ought to be closed (here and elsewhere). Otherwise we'll potentially be holding connections open on the client and server side.
This is probably much more important inside a retryUntilTrue
invocation, as the frequency of retries could be high.
Lombok's @Cleanup
annotation would be a cheap and easy way to ensure that these response objects get cleaned up.
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.
@daschl ping
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.
sorry that I've been slow on this. Working on it now, will update the PR
… primary index The motivation for this change is that in slow environments, it could be that the query engine is not aware of the just created bucket yet and so the create primary index will take longer than the 10s read timeout for the subsequent http request. As a result, this is just another precaution to minimize the chances that in slow environments the create primary index call times out with a read timeout.
b78068d
to
cd1543e
Compare
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.
Awesome, thanks @daschl!
…creating… (testcontainers#2662)" This reverts commit 835ac71.
… primary index
The motivation for this change is that in slow environments, it could be that the
query engine is not aware of the just created bucket yet and so the create primary
index will take longer than the 10s read timeout for the subsequent http request.
As a result, this is just another precaution to minimize the chances that in
slow environments the create primary index call times out with a read timeout.