Skip to content
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

Fix thundering herd problem with debouncer #1230

Merged
merged 5 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/1230.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix an issue where the QuitDebouncer would reprocess old QUITs, and process QUITs too early during the debouncing process.
17 changes: 17 additions & 0 deletions spec/unit/IrcServer.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,23 @@
const { IrcServer } = require("../../lib/irc/IrcServer");
const extend = require("extend");
describe("IrcServer", function() {
describe("getQuitDebounceDelay", () => {
it("should get a random period between min and max", () => {
const delayMinMs = 5;
const delayMaxMs = 10;
const server = new IrcServer("irc.foobar",
extend(true, IrcServer.DEFAULT_CONFIG, {
quitDebounce: {
delayMinMs,
delayMaxMs,
}
})
);
const delay = server.getQuitDebounceDelay();
expect(delay).toBeGreaterThan(delayMinMs);
expect(delay).toBeLessThan(delayMaxMs);
});
})
describe("getNick", function() {
it("should get a nick from a userid", function() {
const server = new IrcServer("irc.foobar",
Expand Down
2 changes: 1 addition & 1 deletion src/bridge/QuitDebouncer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class QuitDebouncer {
// Nobody to debounce, yay.
return;
}
const delay = Math.max(server.getQuitDebounceDelayMinMs(), server.getQuitDebounceDelayMaxMs() * Math.random());
const delay = server.getQuitDebounceDelay();
log.info(`Will attempt to reconnect users for ${channel} after ${delay}ms`)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line uses delay which is no longer defined.

setTimeout(() => {
// Clear our existing sets, we're about to operate on the channel.
Expand Down
23 changes: 7 additions & 16 deletions src/irc/IrcServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,23 +95,14 @@ export class IrcServer {
}

/**
* Get the minimum number of ms to debounce before bridging a QUIT to Matrix
* during a detected net-split. If the user rejoins a channel before bridging
* the quit to a leave, the leave will not be sent.
* @return {number}
*/
public getQuitDebounceDelayMinMs() {
return this.config.quitDebounce.delayMinMs;
}

/**
* Get the maximum number of ms to debounce before bridging a QUIT to Matrix
* during a detected net-split. If a leave is bridged, it will occur at a
* random time between delayMinMs (see above) delayMaxMs.
* @return {number}
* Get a random interval to delay a quits for when debouncing. Will be between
* `delayMinMs` and `delayMaxMs`
*/
public getQuitDebounceDelayMaxMs() {
return this.config.quitDebounce.delayMaxMs;
public getQuitDebounceDelay(): number {
const { delayMaxMs, delayMinMs } = this.config.quitDebounce;
return delayMinMs + (
delayMaxMs - delayMinMs
) * Math.random();
}

/**
Expand Down