Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Wake up transaction queue when remote server comes back online #6706

Merged
merged 10 commits into from
Jan 17, 2020

Conversation

erikjohnston
Copy link
Member

This is a bit fiddly to do as we might only notice a remote is back up on a worker, and the federation sending may or may not be happening on another worker.

Basically:

  1. We poke the notifier when we see a remote is back up, which pokes the transaction queue if on the same process.
    1. If on master then the replication resource also gets poked by the notifier, triggering it to broadcast a REMOTE_SERVER_UP replication command to workers.
    2. If on a worker then we use the replication client to send a REMOTE_SERVER_UP to master, which then triggers the notifier to be called on master too (and in turn causes it to send the command to the the workers)

(Note: this means that the original worker will get a REMOTE_SERVER_UP echoed back to it, but that is not actually problematic so long as we don't then trigger another REMOTE_SERVER_UP. This is why the notifier doesn't poke replication clients on workers)

This will be used to retry outbound transactions to a remote server if
we think it might have come back up.
@erikjohnston erikjohnston force-pushed the erikj/add_remote_server_up branch from a32aa3b to 4474b8f Compare January 15, 2020 16:18
@erikjohnston erikjohnston force-pushed the erikj/add_remote_server_up branch from 4474b8f to 1bd1d4d Compare January 15, 2020 16:22
@erikjohnston erikjohnston requested a review from a team January 15, 2020 16:22
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

looks generally plausible. Can you add the new command to docs/tcp_replication.md please

"""Called when we want to retry sending transactions to a remote.

This is mainly useful if the remote server has been down and we think it
migtht have come back.
Copy link
Member

Choose a reason for hiding this comment

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

migtht

Comment on lines 498 to 501
if destination == self.server_name:
logger.info("Not waking up ourselves")
return

Copy link
Member

Choose a reason for hiding this comment

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

it feels like either we should consider this happening as a sign of badness elsewhere (and log it at more than info), or decide that it's not worth worrying about at all. The halfway house of logging at info seems odd.

@erikjohnston erikjohnston merged commit a8a50f5 into develop Jan 17, 2020
@erikjohnston erikjohnston deleted the erikj/add_remote_server_up branch February 5, 2020 17:35
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'a8a50f5b5':
  Wake up transaction queue when remote server comes back online (#6706)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants