feat: adapt to eloqstore local standby feature#451
Merged
thweetkomputer merged 7 commits intoeloqdata:mainfrom Mar 17, 2026
Merged
feat: adapt to eloqstore local standby feature#451thweetkomputer merged 7 commits intoeloqdata:mainfrom
thweetkomputer merged 7 commits intoeloqdata:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdated the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
eloqkv.ini (1)
179-194: Move LOCAL STANDBY configuration block outside CLOUD MODE section; these configs are not yet implemented in the codebase.The LOCAL STANDBY block is placed within the "CLOUD MODE CONFIGURATION" section (lines 136-140), creating confusion since line 180 explicitly states to "keep this disabled in cloud mode." Additionally, these configuration keys are not referenced anywhere in the codebase, indicating this is forward-looking documentation for a feature not yet implemented.
To improve clarity:
- Move the LOCAL STANDBY block outside the "CLOUD MODE CONFIGURATION" section, or add a clear subsection header like
# >>>>>>>>>>>>> LOCAL STANDBY (NON-CLOUD) <<<<<<<<<<<<<- Consider adding validation guidance for when the feature is implemented:
- What happens if
eloq_store_standby_master_snapshot_rootscount doesn't match the actual snapshot roots on the master?- What happens if
eloq_store_standby_master_store_path_weightscount doesn't match the master's store path list?- Clarify the
host_name:/pathformat when the host includes a port (e.g.,host:port:/pathvshost:/path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eloqkv.ini` around lines 179 - 194, Move the LOCAL STANDBY configuration block (keys eloq_store_enable_local_standby, eloq_store_standby_master_path, eloq_store_standby_master_snapshot_roots, eloq_store_standby_master_store_path_weights) out of the "CLOUD MODE CONFIGURATION" section or prepend a distinct subsection header like "LOCAL STANDBY (NON-CLOUD)" to avoid implying cloud compatibility; mark these keys as not yet implemented in the codebase, and add short TODO/validation notes indicating required behavior once implemented (e.g., how mismatched counts for eloq_store_standby_master_snapshot_roots vs master snapshot roots and eloq_store_standby_master_store_path_weights vs master store paths should be handled), and clarify the host:path format rules including how to represent a host with a port (explicitly state expected syntax such as username@host:port:/absolute/path or an alternative).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@eloqkv.ini`:
- Around line 179-194: Move the LOCAL STANDBY configuration block (keys
eloq_store_enable_local_standby, eloq_store_standby_master_path,
eloq_store_standby_master_snapshot_roots,
eloq_store_standby_master_store_path_weights) out of the "CLOUD MODE
CONFIGURATION" section or prepend a distinct subsection header like "LOCAL
STANDBY (NON-CLOUD)" to avoid implying cloud compatibility; mark these keys as
not yet implemented in the codebase, and add short TODO/validation notes
indicating required behavior once implemented (e.g., how mismatched counts for
eloq_store_standby_master_snapshot_roots vs master snapshot roots and
eloq_store_standby_master_store_path_weights vs master store paths should be
handled), and clarify the host:path format rules including how to represent a
host with a port (explicitly state expected syntax such as
username@host:port:/absolute/path or an alternative).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59fe4e5f-2afb-4dcb-9cde-7ba3cf5bc8d6
📒 Files selected for processing (2)
data_substrateeloqkv.ini
thweetkomputer
force-pushed
the
feat-eloqstore-local-standby-zc
branch
from
42fc632 to
7a511ed
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
eloqkv.ini (1)
192-194: Make standby weight cardinality explicit.Please explicitly state that
eloq_store_standby_master_store_path_weightscount must equaleloq_store_standby_master_store_pathscount (similar to Line 96-97), to prevent misconfiguration ambiguity.Suggested wording
-# [LOCAL STANDBY OPTIONAL] Store path weights for the standby master, separated by ','. -# Use the same ordering as the standby master's store path list. +# [LOCAL STANDBY OPTIONAL] Store path weights for the standby master, separated by ','. +# The number of weights must match eloq_store_standby_master_store_paths. +# Use the same ordering as the standby master's store path list. # eloq_store_standby_master_store_path_weights=🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eloqkv.ini` around lines 192 - 194, Update the comment for eloq_store_standby_master_store_path_weights to explicitly state that the number of comma-separated weights must match the number of entries in eloq_store_standby_master_store_paths (i.e., they must have the same cardinality), mirroring the guidance used earlier for the primary store paths; mention the requirement clearly so users know to provide one weight per standby store path and note that omission or mismatch is invalid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@eloqkv.ini`:
- Around line 192-194: Update the comment for
eloq_store_standby_master_store_path_weights to explicitly state that the number
of comma-separated weights must match the number of entries in
eloq_store_standby_master_store_paths (i.e., they must have the same
cardinality), mirroring the guidance used earlier for the primary store paths;
mention the requirement clearly so users know to provide one weight per standby
store path and note that omission or mismatch is invalid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e58a93d-a5e1-4bfc-8810-6f6fca15004f
📒 Files selected for processing (2)
data_substrateeloqkv.ini
✅ Files skipped from review due to trivial changes (1)
- data_substrate
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@eloqkv.ini`:
- Around line 176-183: The LOCAL STANDBY options
(eloq_store_enable_local_standby, eloq_store_standby_master_addr) are
ambiguously placed and under-documented: move or re-section them out of the
"CLOUD MODE CONFIGURATION" block (or add a clear subsection header "LOCAL
STANDBY (optional, only for non-cloud or advanced use)") and update both entries
to match file conventions—document eloq_store_enable_local_standby as a boolean
with allowed values (e.g., "true/on" to enable, "false/off" to disable) and a
short note that it should normally remain disabled in managed cloud mode;
document eloq_store_standby_master_addr with explicit format examples and rules
(username@host[:port], supporting hostname or IP for host_addr, port optional;
show example like "replica@10.0.0.5:5432" and state behavior when set: standby
will sync from that master instead of cloud storage).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4afd560d-8855-4a93-98ef-7d33145b497c
📒 Files selected for processing (2)
data_substrateeloqkv.ini
✅ Files skipped from review due to trivial changes (1)
- data_substrate
thweetkomputer
force-pushed
the
feat-eloqstore-local-standby-zc
branch
from
06fe140 to
12604fc
Compare
thweetkomputer
force-pushed
the
feat-eloqstore-local-standby-zc
branch
from
6d0ec13 to
85c5897
Compare
fix wip fix wip update config update prolang timeout update test fix update update update update update update
thweetkomputer
force-pushed
the
feat-eloqstore-local-standby-zc
branch
from
91bee0f to
136ff33
Compare
MrGuin
approved these changes
Mar 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit