-
Notifications
You must be signed in to change notification settings - Fork 1.4k
file_chunk: add stricter checks for broken meta files #4998
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
Conversation
2487088
to
ae638cb
Compare
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This metadata could be handled as a hash(map) like data structures.
So, this is just my two cents but it seems that we can add shasum hash for record contents into that metadata.
is it feasible?
This reverts commit f40c07f. Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
I think metadata is extensible. |
Thanks @cosmo0920 ! It would certainly be technically feasible to introduce a checksum for the chunk in the metadata. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM!
Which issue(s) this PR fixes:
What this PR does / why we need it:
This PR improves meta file corruption checking.
The meta file contains at least the following field values.
fluentd/lib/fluent/plugin/buffer/file_chunk.rb
Lines 249 to 254 in fa2eb58
This PR reinforces #1874.
Without this changes, it might causes following error when launch fluentd every time with broken meta file:
If the timekey value is corrupted, the above error occurs.
Since there is no appropriate way to check timekey directly, check
id
,c
, andm
fields instead. This is because when timekey is broken, other fields may also be broken.It might be possible that the
@size
is 0.@unique_id
,@created_at
, and@modified_at
are set when FileChunk is initialized, so they definitely have some values.I think these fields should be written in meta file.
So, this PR adds the
id
,c
, andm
fields check.Previously, it operates using default value if metadata was broken.
However, it can miss the corruption and result in unexpected errors.
So, this PR enhances the detection of broken metadata files instead of using defalut value.
This change has backward compatible with v0.14 behavior.
Docs Changes:
Not necessarily required.
Release Note:
buf_file: reinforce buffer file corruption check