Skip to content

fix(options): Add buffer sizes to failover. Update README#3468

Merged
ndyakov merged 4 commits into
masterfrom
ndyakov/buffer-size-failover
Aug 11, 2025
Merged

fix(options): Add buffer sizes to failover. Update README#3468
ndyakov merged 4 commits into
masterfrom
ndyakov/buffer-size-failover

Conversation

@ndyakov

@ndyakov ndyakov commented Aug 11, 2025

Copy link
Copy Markdown
Member
  • add missing buffer size to failover options
  • half the default buffer size
  • add section in readme related to buffer sizes

Fixes #3465

@ndyakov ndyakov requested a review from ofekshenawa August 11, 2025 09:56
@ndyakov ndyakov added the bug label Aug 11, 2025
@htemelski-oss htemelski-oss self-requested a review August 11, 2025 10:23
Comment thread sentinel.go Outdated
Comment thread sentinel.go Outdated
Comment thread sentinel.go Outdated
Comment thread sentinel.go Outdated
htemelski-oss
htemelski-oss previously approved these changes Aug 11, 2025

@htemelski-oss htemelski-oss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks, good there are two minor things to change but they are already noted

@ofekshenawa ofekshenawa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@ndyakov ndyakov merged commit 94cfffa into master Aug 11, 2025
47 of 48 checks passed
@htemelski-oss htemelski-oss deleted the ndyakov/buffer-size-failover branch August 11, 2025 13:31
@cxljs

cxljs commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

Hello @ndyakov , Here's a small suggestion: How about keeping the read/write buffer size of the sentinel client at 4KB? Since there's no need for the sentinel client to handle large data transfers or pipeline scenarios.

@ndyakov

ndyakov commented Aug 14, 2025

Copy link
Copy Markdown
Member Author

@cxljs great idea! Feel free to work on that or open a separate issue so we can pick it up when possible.

@cxljs

cxljs commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

OK, I'll submit a PR later.

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.

Potential memory leaks on 9.12.0

4 participants