only flush binlog if buffered position has not advanced#581
only flush binlog if buffered position has not advanced#581morgo merged 8 commits intoblock:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reduces the frequency of binary log flushing in the BlockWait function to improve performance and reduce contention during replication.
Changes:
- Modified
BlockWaitto only flush binary logs when the buffered position has not advanced between iterations - Increased the sleep duration between iterations from 100ms to 1 second
- Added tracking of the previous buffered position to detect when progress has stalled
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
eduardo-landin-mestre
force-pushed
the
elandin-issue-565
branch
from
f31f808 to
77d8927
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| i++ | ||
| } | ||
| }() | ||
| time.Sleep(3 * time.Second) // should be enough for BlockWait to block for 1 iteration before catching up, but not guaranteed |
There was a problem hiding this comment.
This test has timing-dependent behavior as acknowledged in the comment "but not guaranteed". The 3-second sleep is meant to allow BlockWait to start and run for at least one iteration, but there's no deterministic way to ensure this happens. If the system is slow or under load, the test could be flaky. Consider using synchronization primitives like channels or wait groups to ensure the test behavior is deterministic, or document this as a known limitation if determinism is not feasible.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
100ns is effectively nothing. So I might as well set it to nothing.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I removed the 100ns sleep in the goroutine insert loop. 100ms is effectively nothing, so I've just made it nothing (as a busy loop). |
A Pull Request should be associated with an Issue.
Fixes #565
As suggested, these changes decrease the frequency with which spirit flushes binary logs by:
FLUSH BINARY LOGSif the buffered position hasn't advanced