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: add memstore config to return failover errors on puts #289

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

samlaf
Copy link
Collaborator

@samlaf samlaf commented Feb 13, 2025

This is useful for testing failover in batcher.

Right now a new proxy would need to be spun up with this config turned on. We should consider adding an admin rpc rpi to turn these on/off dynamically.

Fixes Issue

Fixes #

Changes proposed

Screenshots (Optional)

Note to reviewers

This is useful for testing failover in batcher.

Right now a new proxy would need to be spun up with this config turned on. We should consider adding an admin rpc rpi to turn these on/off dynamically.
@samlaf samlaf changed the title chore: add memstore config to return failover errors on puts feat: add memstore config to return failover errors on puts Feb 13, 2025
ethenotethan
ethenotethan previously approved these changes Feb 13, 2025
Copy link
Collaborator

@ethenotethan ethenotethan left a comment

Choose a reason for hiding this comment

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

LGTM - just one nit

@@ -67,18 +76,25 @@ func CLIFlags(envPrefix, category string) []cli.Flag {
},
&cli.DurationFlag{
Name: PutLatencyFlagName,
Usage: "Artificial latency added for memstore backend to mimic EigenDA's dispersal latency.",
Usage: "(FOR TESTING) Artificial latency added for memstore backend to mimic EigenDA's dispersal latency.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit - isn't this entire construction used for testing? maybe less redundant to put disclosure in category description instead

Copy link
Collaborator Author

@samlaf samlaf Feb 13, 2025

Choose a reason for hiding this comment

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

Good point haha, removed
55b4ec1

Copy link
Collaborator

@litt3 litt3 left a comment

Choose a reason for hiding this comment

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

LGTM

@samlaf samlaf merged commit e5326bd into main Feb 13, 2025
10 checks passed
@samlaf samlaf deleted the chore--add-memstore-config-to-return-failover-errors branch February 13, 2025 17:15
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