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 queue details view showing stale Redis stats #219

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Rua-Yuki
Copy link

@Rua-Yuki Rua-Yuki commented May 8, 2020

This PR fixes #218.

Rather than relying on inconsistent behaviour which ioredis no longer appears to demonstrate, the resulting data from issuing an INFO command is parsed directly.

As an alternative to recreating the wheel with an info parser here, we could always rely on a library such as node-redis-info to have things handled.

The ioredis client used by bull makes no guarantees that the
`serverInfo` field be updated upon issuing an INFO command, and in fact
does not appear to do so (given bull 3.13.0 and ioredis 4.16.3).

While the `serverInfo` field is indeed populated after the initial
connection is established, it is not updated by calling INFO as assumed
with the previous logic. This results in stale data being presented by
Arena.

Rather than relying on this undocumented and evidently fragile
behaviour, we can instead parse the result of the INFO command on our
own.

This also happens to step around an issue with the way that ioredis
parses key-value data in handling the INFO response, where values
containing the colon character are incorrectly trimmed.

Given the stability and superb backwards-compatibility of Redis, this
should be a highly reliable solution.
@skeggse
Copy link
Member

skeggse commented Aug 5, 2020

It'd be nice if ioredis exposed their existing parsing mechanism so we don't have to choose a reimplementation strategy :\

@bradvogel
Copy link
Contributor

@Rua-Yuki Mind reviewing #459 that is similar to this implementation? If that looks OK, I'd like to merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Queue details view shows stale Redis statistics
3 participants