redis / redis/node-redis

Upgrade from 3.1.2 to 4 has broken message delivery

Open
#2,151 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Bug
Dominant language
TypeScript
Stars
17.6k
Forks
2k
Avg merge
2d 3h
Merged PRs (30d)
40

Description

Environment:

  • Node.js Version: 17
  • Redis Server Version: v=6.2.3
  • Node Redis Version: 4.1.0
  • Platform: Dev platform: Release Linux Mint 20.3 Una 64-bit Prod platform: Any Linux

This is a continuation from https://github.com/redis/node-redis/issues/1870 (@leibale you were working on that). We had to release with version 3.1.2 (our existing version) as we couldn't keep consumers waiting any longer. Now circling back to work out what the actual issue is in hope that we can upgrade node redis.

At this stage it's hard to tell. We're not getting any errors. We've upgraded node redis to the latest now (4.1.0), read the Changelog again, been over the v3 to v4 MIgration Guide many times and made the required changes, also in line with createClient configuration.

This is our consolidated diff between the code that works with 3.1.2 (main branch) and the upgraded code to satisfy 4.1.0 requirements (binarymist/upgrade-incl-redis branch).

The most obvious place to be focussing on is in the testerWatcher.

We have two different communication techniques from our CLI to the orchestrator (Server Sent Events (sse), and long polling (lp)). The sse appears to be working fine with the upgrade of node redis, but the lp is not.

The change you see in the get.js is only applicable to sse so I don't think there's any issue in get.js.

We have multiple Test Sessions (each relly on their specific redis channel. The first one seems to work). The changes to the orchestrator.js are unlikely to be an issue if one of the redis channels is working, which leaves our foucs to the testerWatcher... and specifically pollTesterMessages.

There is either something we've somehow missed changing with the upgrade, although I can't see this being the case as we've gone over your upgrade path a good number of times over the last few months. Or there is a bug in node redis. Either way it needs to be fixed in order for us to upgrade node redis.

A high level overview of our architecture can be seen here.

Thanks.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the testerWatcher, especially pollTesterMessages, and compare the working 3.1.2 code with the 4.1.0 upgrade diff; get.js is reported as relevant only to SSE, while orchestrator.js appears less likely. Trace Redis delivery across the separate test-session channels and long-polling path; done means all sessions receive messages under node-redis 4.1.0 without silent failures.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, redis
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.