fix: Do not await full setup to configure dropping replication slot #2137
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
robacourt
approved these changes
Dec 10, 2024
| @@ -134,7 +134,6 @@ defmodule Electric.Connection.Manager do | |||
| end | |||
|
|
|||
| def drop_replication_slot_on_stop(server) do | |||
| await_active(server) | |||
There was a problem hiding this comment.
If you're not calling await_active here, I imagine you're going to need to call await_active in this test in stratovolt to get it to pass.
There was a problem hiding this comment.
Not really because the setup of the test ensures that the stack is :ready - that encapsulates awaiting for the connection manager to be ready
Currently that's a concern in many cases where the shut down doesn't go as planned, so we need a cleanup procedure anyway independent of Electric. But yeah currently it times out if for example it's waiting for the lock connection - no reason to time out that call as that is not the intent of the call. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Encountered an error in cloud work where the call to configure dropping the replication slot would time out because the connection manager would not become
active- since this call only sets a configuration parameter, I think that the connection manager should handle the slot drop internally.I've unblocked the configuration call, and made it so that it just logs a warning if a connection pool is not available to drop the slot.