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

v4 resubscribe not working #1764

Closed
Redjik opened this issue Dec 5, 2021 · 2 comments
Closed

v4 resubscribe not working #1764

Redjik opened this issue Dec 5, 2021 · 2 comments
Labels

Comments

@Redjik
Copy link

Redjik commented Dec 5, 2021

Resubscribe for sub client hangs awaiting Promise.all(promises) in the following part of source code

            const resubscribePromise = this.#queue.resubscribe();
            if (resubscribePromise) {
                promises.push(resubscribePromise);
            }

            if (promises.length) {
                this.#tick(true);
                await Promise.all(promises);
            }

Here's the test code for this:

import {createClient} from "./packages/client";


const pubClient = createClient({
    url: `redis://127.0.0.1:6379`,
    socket: { connectTimeout: 1000 },
})

const subClient = pubClient.duplicate();

pubClient.on('error', (error: Error) => console.error(error));
subClient.on('error', (error: Error) => console.error(error));

pubClient.on('connect', () => console.log('pubClient connected and starting initiator'));
pubClient.on('ready', () => console.log('pubClient is ready'));
subClient.on('connect', () => console.log('subClient connected and starting initiator'));
subClient.on('ready', () => console.log('subClient is ready'));

(async () => {
   await pubClient.connect();
   await subClient.connect();

   subClient.subscribe('some-key', () => {
       console.log('Received');
   });

   setInterval(() => {
       pubClient.publish('some-key', 'data');
       console.log('Sent');
   },  10 * 1000)
})()

So I just run the test script and restart docker container.

Environment:

  • Node.js Version: 12.22.7
  • Redis Server Version: 6.2.6
  • Node Redis Version: 4.0.0
  • Platform: MacOS
@Redjik Redjik added the Bug label Dec 5, 2021
@Redjik
Copy link
Author

Redjik commented Dec 5, 2021

I might try to dig a bit deeper, will post updates in the comments.

@Redjik
Copy link
Author

Redjik commented Dec 5, 2021

looks like there's some kind of race condition
socket instantly reconnects (receives connect event, though redis is still down, dont know why this happens) and on the next event loop tick it gets close event
during those 2 ticks initiator starts resubscribe tasks but they belong to the previous socket instance

@leibale leibale closed this as completed in 82920ae Dec 7, 2021
leibale added a commit that referenced this issue Dec 13, 2021
* update workflows & README

* add .deepsource.toml

* fix client.quit, add error events on cluster, fix some "deepsource.io" warnings

* Release 4.0.0-rc.1

* add cluster.duplicate, add some tests

* fix #1650 - add support for Buffer in some commands, add GET_BUFFER command

* fix GET and GET_BUFFER return type

* update FAQ

* Update invalid code example in README.md (#1654)

* Update invalid code example in README.md

* Update README.md

Co-authored-by: Leibale Eidelman <leibale1998@gmail.com>

* fix #1652

* ref #1653 - better types

* better types

* fix 5412479

* Update GEOSEARCHSTORE.spec.ts

* fix #1660 - add support for client.HSET('key', 'field', 'value')

* upgrade dependencies, update README

* fix #1659 - add support for db-number in client options url

* fix README, remove unused import, downgrade typedoc & typedoc-plugin-markdown

* update client-configurations.md

* fix README

* add CLUSTER_SLOTS, add some tests

* fix "createClient with url" test with redis 5

* remove unused imports

* Release 4.0.0-rc.2

* add missing semicolon

* replace empty "transformReply" functions with typescript "declare"

* fix EVAL & EVALSHA, add some tests, npm update

* fix #1665 - add ZRANGEBYLEX, ZRANGEBYSCORE, ZRANGEBYSCORE_WITHSCORES

* new issue templates

* add all COMMAND commands

* run COMMAND & COMMAND INFO tests only on redis >6

* Create SECURITY.md

* fix #1671 - add support for all client configurations in cluster

* ref #1671 - add support for defaults

* remove some commands from cluster, npm update, clean code,

* lock benny version

* fix #1674 - remove `isolationPoolOptions` when creating isolated connection

* increase test coverage

* update .npmignore

* Release 4.0.0-rc.3

* fix README

* remove whitespace from LICENSE

* use "export { x as y }" instead of import & const

* move from "NodeRedis" to "Redis"

* fix #1676

* update comments

* Auth before select database (#1679)

* Auth before select database

* fix #1681

Co-authored-by: leibale <leibale1998@gmail.com>

* Adds connect-as-acl-user example. (#1684)

* Adds connect-as-acl-user example.

* Adds blank line at end.

* Set to private.

* Adds examples folder to npmignore.

* Adds Apple .DS_Store file to .gitignore (#1685)

* Adds Apple .DS_Store file.

* Add .DS_Store to .npmignore too

Co-authored-by: Leibale Eidelman <leibale1998@gmail.com>

* move examples

* clean some tests

* clean code

* Adds examples table of contents and contribution guidelines. (#1686)

* Updated examples to use named functions. (#1687)

* Updated examples to user named functions.

* Update README.md

Co-authored-by: Leibale Eidelman <leibale1998@gmail.com>

* update docs, add 6.0.x to the tests matrix, add eslint, npm update, fix some commands, fix some types

Co-authored-by: Simon Prickett <simon@crudworks.org>

* fix tests with redis 6.0.x

* fix ACL GETUSER test

* fix client.quit and client.disconnect

* fix ACL GETUSER

* Adds TypeScript note and corrects a typo.

* Fixes a bug in the Scan Iterator section. (#1694)

* Made examples use local version.

* Add `lua-multi-incr.js` example (#1692)

Also fix syntax error in the lua example in the README

Closes #1689.

* Add(examples): Create an example for blPop & lPush (#1696)

* Add(examples): Create an example for blPop & lPush

Signed-off-by: Aditya Rastogi <adit.rastogi2014@gmail.com>

* Update(examples): fix case, add timeout, update readme

Signed-off-by: Aditya Rastogi <adit.rastogi2014@gmail.com>

Closes #1693.

* Add command-with-modifiers.js example (#1695)

* Adds TypeScript note and corrects a typo.

* Adds command-with-modifiers example. (#1688)

* Adds command-with-modifiers example. (#1688)

* Adds command-with-modifiers example. (#1688)

* Removed callbacks.

Co-authored-by: Simon Prickett <simon@redislabs.com>

Closes #1688.

* Issue # 1697 FIX - creates an example script that shows how to use the SSCAN iterator (#1699)

* #1697 fix for set scan example

* adds the js file

* adds comment

* Minor layout and comment adjustment.

Co-authored-by: srawat2 <shashank19aug>
Co-authored-by: Simon Prickett <simon@redislabs.com>

Closes #1697.

* fix #1706 - HSET return type should be number

* use dockers for tests, fix some bugs

* increase dockers timeout to 30s

* release drafter (#1683)

* release drafter

* fixing contributors

* use dockers for tests, use npm workspaces, add rejson & redisearch modules, fix some bugs

* fix #1712 - fix LINDEX return type

* uncomment TIME tests

* use codecov

* fix tests.yml

* uncomment "should handle live resharding" test

* fix #1714 - update README(s)

* add package-lock.json

* update CONTRIBUTING.md

* update examples

* uncomment some tests

* fix test-utils

* move "all-in-one" to root folder

* fix tests workflow

* fix bug in cluster slots, enhance live resharding test

* fix live resharding test

* fix #1707 - handle number arguments in legacy mode

* Add rejectedUnauthorized and other TLS options (#1708)

* Update socket.ts

* fix #1716 - decode username and password from url

* fix some Z (sorted list) commands, increase commands test coverage

* remove empty lines

* fix 'Scenario' typo (#1720)

* update readmes, add createCluster to the `redis` package

* add .release-it.json files, update some md files

* run tests on pull requests too

* Support esModuleInterop set to false. (#1717)

* Support esModuleInterop set to false.

When testing the upcoming 4.x release, we got a bunch of typescript
errors emitted from this project.

We quickly realized this is because the library uses the esModuleInterop
flag. This makes some imports _slightly_ easier to write, but it comes
at a cost: it forces any application or library using this library to
*also* have esModuleInterop on.

The `esModuleInterop` flag is a bit of a holdover from an earlier time,
and I would not recommend using it in libraries. The main issue is that
if it's set to true, you are forcing any users of the library to also
have `esModuleInterop`, where if you keep have it set to `false` (the
default), you leave the decision to the user.

This change should have no rammifications to users with
`esModuleInterop` on, but it will enable support for those that have it
off.

This is especially good for library authors such as myself, because I
would also like to keep this flag off to not force *my* users into this
feature.

* All tests now pass!

* Move @types/redis-parser into client sub-package

and removed a comma

* npm update, remove html from readme

* add tests and licence badges

* update changelog.md

* update .npmignore and .release-it.json

* update .release-it.json

* Release client@1.0.0-rc.0

* revert d32f1ed

* fix .npmignore

* replace @redis with @node-redis

* Release client@1.0.0-rc.0

* update json & search version

* Release json@1.0.0-rc.0

* Release search@1.0.0-rc.0

* update dependencies

* Release redis@4.0.0-rc.4

* fix #1724 - fix LINDEX signature

* add positive test for LINDEX

* fix #1718 - add support for buffers in pubsub

* Fixed a few typos.

* fix ARRPOP

* fix #1726

* enhance cluster reshard handling

* Adds RediSearch demo.

* Adds intro sentence.

* Made top level comment more descriptive.

* Adds RedisJSON example.

* Renamed JSON search example.

* Some refactoring.

* Fixed search example for JSON.

* Minor wording updates.

* Added missing pet name.

* Adds JSON package overview.

* Fixed typo.

* Search package README initial version.

* remove echo from docker entrypoint.sh

* npm update

* update docs

* fix merge

* fix merge

* Release client@1.0.0

* npm update

* Release search@1.0.0

* update sub modules

* Release redis@4.0.0

* update README

* Fixed `zScanIterator` example (#1733)

The example for `zScanIterator` had the wrong values in the `for` loop.

* fix #1738 - add support for buffers in HSET

* fix for c96a0dd - remove .only from HSET tests

* increase HSET test coverage

* fix for d0de622

* ref #1741 - fix socket options type

* fix #1741 - change default to `localhost` and update docs

* fix #1739 - add support for number as value in HSET

* fix tests workflow

* increase pushGeoCountArgument test coverage

* init time series

* Update v3-to-v4.md (#1737)

* Update v3-to-v4.md

* Update v3-to-v4.md

* Update v3-to-v4.md

* Update docs/v3-to-v4.md

Co-authored-by: Simon Prickett <simon@crudworks.org>

* Update docs/v3-to-v4.md

Co-authored-by: Simon Prickett <simon@crudworks.org>

* Update docs/v3-to-v4.md

Co-authored-by: Simon Prickett <simon@crudworks.org>

* Update docs/v3-to-v4.md

Co-authored-by: Simon Prickett <simon@crudworks.org>

* Update docs/v3-to-v4.md

Co-authored-by: Simon Prickett <simon@crudworks.org>

* Update docs/v3-to-v4.md

Co-authored-by: Simon Prickett <simon@crudworks.org>

Co-authored-by: Leibale Eidelman <leibale1998@gmail.com>
Co-authored-by: Simon Prickett <simon@crudworks.org>

* fix #1744 - add npm minimum version

* fix #1743 - restrict the list of published files (#1742)

* fix TS.CREATERULE

* fix #1673 - Export RedisClientType and RedisClusterType types (#1704)

* add description to package.json

* fix #1749 - FT.SEARCH SORTBY

* Client: Export errors (#1750)

* Client: Export errors

* export RedisModules & RedisScripts as well

Co-authored-by: Leibale Eidelman <leibale1998@gmail.com>

* fix for 592714f - "fix tests"

* fix for a453114 - fix FT.SEARCH

* fix FT.SEARCH

* improve encodeCommand performance, add set-get-delete-string benchmark

* fix encodeCommand

* Add the `event` doc (#1753)

* hdr-histogram-js

* fix #1764 - fix PubSub resubscribe

* fix #1758 - implement some CLIENT commands, add `name` to `RedisClientOptions`

* update tests badge link

* update license badge link

* ref #1765 - support lowercase command names in legacy mode

* update benchmarks

* fix LINSERT test title

* fix v4 benchmarks

* fix discord link

* fix #1771 - fix PubSub resubscribe

* fix #1766 - allow .quit() in PubSub mode

* fix for e112568 - should not retry connecting if failed due to wrong auth

* Add LGTM (#1773)

* Update README.md

* Update README.md

Co-authored-by: Leibale Eidelman <leibale1998@gmail.com>

* fix #1734 - fix PubSub unsubscribe race condition

* Support RedisTimeSeries (#1757)

* Implement missing commands and add test

* Update DECRBY.spec.ts

* Small changes

* clean code

* Update MGET_WITHLABELS.ts

Use map in transformReply

Co-authored-by: leibale <leibale1998@gmail.com>

* fix #1774 #1767 - update docs

Co-authored-by: Chayim <chayim@users.noreply.github.com>

* Search commands (#1778)

* ft.alter

* ft.profile

* clean code

* fix generated documentation

* fix documentation workflow

* fix documentation workflow

* fix documentation workflow

* update CHANGELOG

* Release client@1.0.1

* update tsconfig.json

* Release json@1.0.1

* Release search@1.0.1

* fix time-series .release-it.json

* add @node-redis/time-series to README

* add time-series to all in one

* Release redis@4.0.1

Co-authored-by: Richard Samuelsson <noobtoothfairy@gmail.com>
Co-authored-by: mustard <mhqnwt@gmail.com>
Co-authored-by: Simon Prickett <simon@redislabs.com>
Co-authored-by: Simon Prickett <simon@crudworks.org>
Co-authored-by: Suze Shardlow <SuzeShardlow@users.noreply.github.com>
Co-authored-by: Joshua T <buildingsomethingfun@gmail.com>
Co-authored-by: Aditya Rastogi <adit.rastogi2014@gmail.com>
Co-authored-by: Rohan Kumar <rohan.kr20@gmail.com>
Co-authored-by: Kalki <shashank.kviit@gmail.com>
Co-authored-by: Chayim <chayim@users.noreply.github.com>
Co-authored-by: Da-Jin Chu <dajinchu@gmail.com>
Co-authored-by: Henrique Corrêa <75134774+HeCorr@users.noreply.github.com>
Co-authored-by: Evert Pot <me@evertpot.com>
Co-authored-by: AnnAngela <naganjue@vip.qq.com>
Co-authored-by: Damien Arrachequesne <damien.arrachequesne@gmail.com>
Co-authored-by: kjlis <kjlis@users.noreply.github.com>
Co-authored-by: Vojta Staněk <stanekv01@gmail.com>
Co-authored-by: Guy Korland <gkorland@gmail.com>
Co-authored-by: Avital Fine <79420960+AvitalFineRedis@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant