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: Fixed autopipeline performances. #1226

Merged
merged 4 commits into from
Jun 13, 2021
Merged

fix: Fixed autopipeline performances. #1226

merged 4 commits into from
Jun 13, 2021

Conversation

ShogunPanda
Copy link
Contributor

What's in there?

This PR introduces new internal structures to compare the target nodes for different slots.
In particular, it assigns a numeric ID to each group of nodes (where by "group" I mean a master and its slaves).
This ID is then searchable using both the group name itself or by slot.

The change is needed to improve autopipeline performances, which degraded after the fix in #1224.

Notes to reviewers

playground.js and .gitignore changes will be removed when the PR is ready for review/merge.

@mcollina
Copy link
Contributor

mcollina commented Nov 9, 2020

Good for me!

lib/cluster/index.ts Outdated Show resolved Hide resolved
luin pushed a commit that referenced this pull request Dec 13, 2020
This issue has also been addressed by others in #1232 and #1226.

Closes #1232.

After calling `redis.quit` there are still many open `setInterval` handles that have not been cleared.

These uncleared `setInternal` appear to be because everytime `connect` is called in the `reconnectTimeout` the `this._addedScriptHashesCleanInterval` is overridden but the old one still exists in the background.

https://github.com/luin/ioredis/blob/v4.19.2/lib/cluster/index.ts#L311-L313

When you call `redis.quit` it only clears the current one retained by `this._addedScriptHashesCleanInterval`.

```
[WTF Node?] open handles:
- File descriptors: (note: stdio always exists)
  - fd 1 (tty) (stdio)
  - fd 0 (tty)
  - fd 2 (tty) (stdio)
- Intervals:
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
  - (60000 ~ 60 s) (anonymous) @ /srv/app/node_modules/ioredis/built/redis/index.js:258
```
ioredis-robot pushed a commit that referenced this pull request Dec 13, 2020
## [4.19.4](v4.19.3...v4.19.4) (2020-12-13)

### Bug Fixes

* prevent duplicate intervals being set. ([#1244](#1244)) ([515d9ea](515d9ea)), closes [#1232](#1232) [#1226](#1226) [#1232](#1232) [/github.com/luin/ioredis/blob/v4.19.2/lib/cluster/index.ts#L311-L313](https://github.com//github.com/luin/ioredis/blob/v4.19.2/lib/cluster/index.ts/issues/L311-L313)
@ShogunPanda
Copy link
Contributor Author

I have rebased this after latest master. Are we good to merge this?

@luin luin merged commit 42f1ee1 into redis:master Jun 13, 2021
ioredis-robot pushed a commit that referenced this pull request Jun 13, 2021
## [4.27.6](v4.27.5...v4.27.6) (2021-06-13)

### Bug Fixes

* fixed autopipeline performances. ([#1226](#1226)) ([42f1ee1](42f1ee1))
@ioredis-robot
Copy link
Collaborator

🎉 This PR is included in version 4.27.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ShogunPanda ShogunPanda deleted the fix-autopipeline-performances branch June 14, 2021 04:45
janus-dev87 added a commit to janus-dev87/ioredis-work that referenced this pull request Mar 1, 2024
## [4.19.4](redis/ioredis@v4.19.3...v4.19.4) (2020-12-13)

### Bug Fixes

* prevent duplicate intervals being set. ([#1244](redis/ioredis#1244)) ([515d9ea](redis/ioredis@515d9ea)), closes [#1232](redis/ioredis#1232) [#1226](redis/ioredis#1226) [#1232](redis/ioredis#1232) [/github.com/luin/ioredis/blob/v4.19.2/lib/cluster/index.ts#L311-L313](https://github.com//github.com/luin/ioredis/blob/v4.19.2/lib/cluster/index.ts/issues/L311-L313)
janus-dev87 added a commit to janus-dev87/ioredis-work that referenced this pull request Mar 1, 2024
## [4.27.6](redis/ioredis@v4.27.5...v4.27.6) (2021-06-13)

### Bug Fixes

* fixed autopipeline performances. ([#1226](redis/ioredis#1226)) ([42f1ee1](redis/ioredis@42f1ee1))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants