Skip to content

Commit

Permalink
fix(core): Perform multi-main leader check against key ID (#7964)
Browse files Browse the repository at this point in the history
## Summary
Current leader check is based on key presence and multi-main instance
type, which can give rise to [this edge
case](https://n8nio.slack.com/archives/C04B1GZ4T0U/p1702025497086379?thread_ts=1701962808.817579&cid=C04B1GZ4T0U)
where leader fails to realize they lost leadership to a former follower.
PR performs the check against the specific key ID instead to prevent
this.

...

#### How to test the change:
1. ...


## Issues fixed
Include links to Github issue or Community forum post or **Linear
ticket**:
> Important in order to close automatically and provide context to
reviewers

...


## Review / Merge checklist
- [ ] PR title and summary are descriptive. **Remember, the title
automatically goes into the changelog. Use `(no-changelog)` otherwise.**
([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md))
- [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up
ticket created.
- [ ] Tests included.
> A bug is not considered fixed, unless a test is added to prevent it
from happening again. A feature is not complete without tests.
  >
> *(internal)* You can use Slack commands to trigger [e2e
tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227)
or [deploy test
instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce)
or [deploy early access version on
Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e).
  • Loading branch information
ivov authored Dec 8, 2023
1 parent 9458255 commit 1a87f70
Showing 1 changed file with 22 additions and 18 deletions.
40 changes: 22 additions & 18 deletions packages/cli/src/services/orchestration/main/MultiMainSetup.ee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,41 +63,45 @@ export class MultiMainSetup extends SingleMainSetup {
}

private async checkLeader() {
if (!this.redisPublisher.redisClient) return;

const leaderId = await this.redisPublisher.get(this.leaderKey);

if (!leaderId) {
this.logger.debug('Leadership vacant, attempting to become leader...');
await this.tryBecomeLeader();
if (leaderId === this.id) {
this.logger.debug(`[Instance ID ${this.id}] Leader is this instance`);

await this.redisPublisher.setExpiration(this.leaderKey, this.leaderKeyTtl);

return;
}

if (this.isLeader) {
this.logger.debug(`Leader is this instance "${this.id}"`);
if (leaderId && leaderId !== this.id) {
this.logger.debug(`[Instance ID ${this.id}] Leader is other instance "${leaderId}"`);

await this.redisPublisher.setExpiration(this.leaderKey, this.leaderKeyTtl);
} else {
this.logger.debug(`Leader is other instance "${leaderId}"`);
if (config.getEnv('multiMainSetup.instanceType') === 'leader') {
this.emit('leadershipChange', leaderId); // stop triggers, pruning, etc.

config.set('multiMainSetup.instanceType', 'follower');
}

return;
}

if (!leaderId) {
this.logger.debug(
`[Instance ID ${this.id}] Leadership vacant, attempting to become leader...`,
);

config.set('multiMainSetup.instanceType', 'follower');

await this.tryBecomeLeader();
}
}

private async tryBecomeLeader() {
if (
config.getEnv('multiMainSetup.instanceType') === 'leader' ||
!this.redisPublisher.redisClient
) {
return;
}

// this can only succeed if leadership is currently vacant
const keySetSuccessfully = await this.redisPublisher.setIfNotExists(this.leaderKey, this.id);

if (keySetSuccessfully) {
this.logger.debug(`Leader is now this instance "${this.id}"`);
this.logger.debug(`[Instance ID ${this.id}] Leader is now this instance`);

config.set('multiMainSetup.instanceType', 'leader');

Expand Down

0 comments on commit 1a87f70

Please sign in to comment.