-
Notifications
You must be signed in to change notification settings - Fork 9.8k
loggerd: add unit tests and fix identified sync issues #34840
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
base: master
Are you sure you want to change the base?
Conversation
Unit tests revealed three flawed assumptions in the encoder-logger synchronization logic, leading to synchronization failures and continuous queue growth, which ultimately exhausted memory (these issues will be fixed later in this PR.):
|
9351c2f
to
fcc7a0c
Compare
fcc7a0c
to
492a959
Compare
resolve: #34839
This PR adds unit tests specifically aimed at resolving synchronization issues between encoderd and loggerd. While existing tests thoroughly cover logger rotation, these new test cases address the previously untested logic for synchronizing encoder segment numbers with logger segment numbers.
Key Changes:
LoggerdState
andRemoteEncoder
classes were moved tologgerd.h
.RemoteEncoder::syncSegment
, which can now be unit tested.Future Improvements:
As part of future work,
loggerd.h
can be split to separate configuration and logic. For instance, renamingloggerd.h
toencoder_config.h
and limitingloggerd.h
to only contain declarations used byloggerd.cc
. However, since this would introduce substantial changes, it will be addressed in a follow-up PR.