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

(max - this.inFlight()) have to be greater than or equal to 0 #404

Open
743v45 opened this issue Apr 25, 2023 · 7 comments · May be fixed by #406
Open

(max - this.inFlight()) have to be greater than or equal to 0 #404

743v45 opened this issue Apr 25, 2023 · 7 comments · May be fixed by #406

Comments

@743v45
Copy link
Contributor

743v45 commented Apr 25, 2023

const RRL = require('./lib/roundrobinlist');

const list = new RRL([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);

console.log(list.next(1));
// [ 1 ]
console.log(list.next(-2));
// [
//   2, 3, 4, 5,
//   6, 7, 8, 9
// ]
@dudleycarr
Copy link
Owner

Might be nice to put in a check for roundrobinlist. However, is there ever a chance that inFlight would be less than zero?

@743v45
Copy link
Contributor Author

743v45 commented Apr 25, 2023

@743v45 743v45 changed the title max - this.inFlight() have to be greater than or equal to 0 (max - this.inFlight()) have to be greater than or equal to 0 Apr 25, 2023
@dudleycarr
Copy link
Owner

Sure, but are you running into a situation where max - this.inFlight() < 0?

@743v45
Copy link
Contributor Author

743v45 commented Apr 25, 2023

const RRL = require('./lib/roundrobinlist');
const list = new RRL([1, 2, 3]);
console.log(list.next(2));
// [ 1, 2 ]
console.log(list.next(2));
// [ 3 ]

roundrobinlist.next is incomplete actually.

@743v45
Copy link
Contributor Author

743v45 commented Apr 25, 2023

Only when the perConnectionMax is 0, it will run here, but this code always rebalances within 50 milliseconds and cannot be observed.

@743v45
Copy link
Contributor Author

743v45 commented Apr 25, 2023

When the current_state_name is TRY_ONE, max is 1 and inFlight() can be 3.

@dudleycarr
Copy link
Owner

OK, I'm convinced :) Let me investigate and work on a fix.

@743v45 743v45 linked a pull request Apr 26, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants