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

feat(db): select Redis DB index on single-instance non-cluster configurations #10

Merged
merged 4 commits into from
Jun 13, 2024

Conversation

rbonestell
Copy link
Contributor

@rbonestell rbonestell commented Jun 13, 2024

I discovered this repository from the original, where I had opened an issue regarding the inability to select a specific Redis DB index for use with Redlock in single-instance configurations with cluster mode disabled: mike-marcacci/node-redlock#298

I realized after browsing other issues and open PRs that the other repository was stale and no longer actively maintained. As a result I found this repo! I am happy to see all of the enhancements and the activity on this repository, and want to contribute my enhancements:

  1. Added db property to Settings class options. Defaults to 0 when omitted or provided with an invalid value.
  2. Updated Lua scripts to use redis.pcall("SELECT", tonumber(ARGV[1])) to attempt to select the DB index, which will silently no-op for configurations which don't support the SELECT command or multiple DB indexes (Redis instances with cluster mode enabled).
    • The SELECT command is specifically executed in each script because I have experienced several instances where using a shared ioredis Redis client instance in my app's dependency container has lead to a race condition where records destined for another DB index end up in the Redlock specific DB index, and vis-versa. Performing the select in the script essentially ensures an atomic execution of the DB select along with the script.
  3. Added unit tests (unit.spec.ts) to project to cover Redlock settings test cases and Redlock acquire(), extend(), and release() use cases with mocks.
  4. Renamed index.spec.ts to system.spec.ts to distinguish system tests from unit tests.
  5. Updated README.md file to document new db settings option and behavior, and to correct invalid example import statements for ioredis (import Client from "ioredis"; does not work with the current version, while import Redis from "ioredis"; does).
  6. Updated yarn.lock to resolve high severity vulnerability with downstream dependency braces:

    braces <3.0.3
    Severity: high
    Uncontrolled resource consumption in braces - GHSA-grv7-fg5c-xmjg

ℹ️ Also note, there is no CONTRIBUTING.md file in this repo despite the README.md file linking to one. (An artifact of the old repository contents, perhaps?)

… index which to use for single-instance configurations with cluster mode disabled
… and added details for configuring redlock to use a specific DB index for single instance with cluster mode disabled
…pec.ts to system.spec.ts to distinguish unit from system tests. Updated package.json accordingly.

Added system tests to cover use cases for configuring a specific DB number.
Added unit test coverage for Redlock settings and acquire, extend, and release methods.
@rbonestell rbonestell marked this pull request as ready for review June 13, 2024 06:35
@djMax djMax changed the title Select Redis DB index on single-instance non-cluster configurations feat(db): select Redis DB index on single-instance non-cluster configurations Jun 13, 2024
@djMax djMax merged commit 39644cf into sesamecare:main Jun 13, 2024
6 checks passed
Copy link

🎉 This PR is included in version 1.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

2 participants