Skip to content

adapter: reduce Lua list lock contention#384

Merged
bootjp merged 10 commits into
mainfrom
feature/reduce-lock
Mar 21, 2026
Merged

adapter: reduce Lua list lock contention#384
bootjp merged 10 commits into
mainfrom
feature/reduce-lock

Conversation

@bootjp
Copy link
Copy Markdown
Owner

@bootjp bootjp commented Mar 21, 2026

No description provided.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the performance and robustness of Redis list operations executed via Lua scripts. By adopting a lazy loading and delta-based commit approach, it drastically reduces the data processed and locked for list modifications, thereby mitigating contention. Furthermore, the transaction retry logic has been made more intelligent, distinguishing between different types of retryable errors and providing clearer error reporting by normalizing internal keys.

Highlights

  • Optimized Redis List Handling in Lua: Implemented a lazy loading and delta-based commit strategy for Redis lists within Lua scripts. This reduces the need to load and write entire lists, minimizing lock contention and improving performance for list operations.
  • Refined Transaction Retry Mechanism: Introduced distinct retry policies for write conflicts and transaction locks, allowing for more granular backoff strategies. Error messages for retryable transaction errors are now normalized to expose user-facing keys instead of internal key representations.
  • Enhanced List State Management: The luaListState struct was extended with fields to track partial list modifications (leftTrim, rightTrim, leftValues, rightValues) and a materialized flag, enabling efficient in-memory manipulation without full data retrieval.
  • New Test Coverage: Added new test cases to validate the behavior of RPOPLPUSH with the optimized list implementation, ensuring correct functionality and TTL preservation.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a significant optimization to reduce lock contention for Lua list operations by implementing a lazy-loading and delta-committing mechanism. Instead of materializing entire lists, operations like LPUSH/RPUSH and LPOP/RPOP now work on deltas, minimizing the transaction's read/write set. The transaction retry logic is also enhanced with different backoff policies for write conflicts versus transaction locks, and error messages are normalized to group retries for the same logical key. The changes are complex but well-implemented and include relevant tests. I've added a couple of suggestions for improving code clarity and robustness.

Comment thread adapter/redis_lua_context.go Outdated
Comment thread adapter/redis_retry.go Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to reduce contention in the Redis adapter’s Lua transaction path, primarily by avoiding full list materialization and by committing list changes as deltas where possible. It also refines retry behavior for retryable transaction errors by using different backoff policies and normalizing internal keys in error messages for clearer diagnostics.

Changes:

  • Add kv.ExtractTxnUserKey to recover logical user keys from !txn|...| internal keys.
  • Update Redis txn retry handling with per-error retry policies and error-key normalization (list keys, TTL keys, txn-internal keys).
  • Rework Lua list handling to lazily load list metadata, materialize only when required, and commit list edits via delta operations; add compatibility tests for RPOPLPUSH behavior and TTL preservation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
kv/txn_keys.go Adds helper to extract logical user key from txn-internal key namespaces.
adapter/redis_retry.go Introduces retry policies and normalizes retryable txn error keys to reduce noisy internal key strings.
adapter/redis_retry_test.go Adds unit tests for retry policy selection and error normalization on internal keys.
adapter/redis_lua_context.go Implements lazy list state, list materialization on-demand, and delta-based list commits to reduce write amplification/lock contention.
adapter/redis_lua_compat_test.go Adds integration coverage for RPOPLPUSH list semantics and TTL preservation in Lua flows.

Comment on lines 2501 to +2505
}

func (c *luaScriptContext) listCommitElems(key string) ([]*kv.Elem[kv.OP], error) {
func (c *luaScriptContext) stringCommitElems(key string) ([]*kv.Elem[kv.OP], error) {
st, err := c.stringState([]byte(key))
if err != nil {
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listCommitPlan returns preserveExisting: true for the non-materialized/delta path, which skips deleteLogicalKeyElems. If a Lua script calls deleteLogical (e.g., DEL key) and then recreates the same key as a list in the same transaction, listState will not load the persisted list meta (because c.deleted[key]=true makes keyType return redisTypeNone). The delta commit then writes only the new meta/items and leaves all previous list items (and potentially other-type internal keys) orphaned in storage. Suggest tracking an "ever deleted/reset" flag per key and forcing a full rewrite (preserveExisting=false) when a key was deleted/reset earlier in the script, even if later recreated as a list.

Copilot uses AI. Check for mistakes.
@bootjp
Copy link
Copy Markdown
Owner Author

bootjp commented Mar 21, 2026

@copilot open a new pull request to apply changes based on the comments in this thread

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 21, 2026

@bootjp I've opened a new pull request, #385, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 3 commits March 21, 2026 09:54
adapter: fix orphaned list items when DEL + recreate in same Lua script
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread adapter/redis_lua_context.go Outdated
bootjp and others added 2 commits March 21, 2026 19:13
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Comment thread store/store.go Outdated
Comment thread kv/txn_errors.go Outdated
bootjp and others added 2 commits March 21, 2026 19:27
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@bootjp bootjp enabled auto-merge March 21, 2026 10:28
@bootjp bootjp merged commit bac2368 into main Mar 21, 2026
8 checks passed
@bootjp bootjp deleted the feature/reduce-lock branch March 21, 2026 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants