Change feature detection for CRDB watch to not require waiting #2205
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead, we purposefully give an invalid parameter, which returns an expected error, but only if watch is supported
vroldanbet
reviewed
Jan 13, 2025
| // Start a changefeed with an invalid value. If we get back an invalid value error (SQLSTATE 22023) | ||
| // then we know that the datastore supports watch. If we get back any other error, then we know that | ||
| // the datastore does not support watch emits or there is a permissions issue. | ||
| _ = cds.writePool.ExecFunc(ctx, func(ctx context.Context, tag pgconn.CommandTag, err error) error { |
There was a problem hiding this comment.
The changestream does not run through any connection pools based on a recommendation from CRL; why is it being done that way here?
not sure if this runs in a loop or a certain cadence, but that would cause connections to be terminated (because of the error)
There was a problem hiding this comment.
Because its going to fail either way; the reason to not use a pool is because it can be a long-running operation
There was a problem hiding this comment.
I meant that it is using a pool and I don't think it should. It could cause connection churn on a pool that is serving write traffic.
There was a problem hiding this comment.
Its only called on startup, so it should be fine
vroldanbet
approved these changes
Jan 13, 2025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead, we purposefully give an invalid parameter, which returns an expected error, but only if watch is supported