fix: ignore chunks too short OR not not seems to pattern .00...N #1615
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some debug output for invalid chunk_name_to_file_name(): 241107 10:12:15 xbcloud: add key: '20241105-shrinked2/ddl_log.log.qp.00000000000000000001' 241107 10:12:15 xbcloud: add key: '20241105-shrinked2/done' 241107 10:12:15 xbcloud: add key: '20241105-shrinked2/ib_buffer_pool.qp.00000000000000000000' -- 241107 10:12:15 xbcloud: Check chunk: '20241105-shrinked2/ddl_log.log.qp.00000000000000000001' 241107 10:12:15 xbcloud: Append chunk: '20241105-shrinked2/ddl_log.log.qp' : 1 241107 10:12:15 xbcloud: Check chunk: '20241105-shrinked2/done' 241107 10:12:15 xbcloud: Append chunk: '20' : 41105 Chunk name '20241105-shrinked2/done' has 23 chars (but not contains '.'), so it splits to filename '20' and idx '241105-shrinked2/done', then atoll cast idx to '241105'.
|
|
| @@ -1038,10 +1038,16 @@ bool xbcloud_put(Object_store *store, const std::string &container, | |||
| @return true in case of success or false otherwise */ | |||
| bool chunk_name_to_file_name(const std::string &chunk_name, | |||
| std::string &file_name, my_off_t &idx) { | |||
| if (chunk_name.size() < 22 && chunk_name[chunk_name.size() - 21] != '.') { | |||
| if (chunk_name.size() < 22 || chunk_name[chunk_name.size() - 21] != '.') { | |||
There was a problem hiding this comment.
This was noted in another #1538. Technically, it is a problem but not seen in the real world because of the minimum prefix length uploaded.
We can accept this.
Comment on lines
+1045
to
+1050
| auto slash_pos = chunk_name.find('/'); | ||
| if (slash_pos && chunk_name.size() - slash_pos < 22) { | ||
| /* '/' must be placed left side from '.' */ | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
I more interested to know about your done file.
- Why do you do it?
- Do you upload 'done' file for every chunk or file?
- Did you modify xbcloud logic to do it? If you are doing externally, do you do it after the backup is uploaded to S3?
There was a problem hiding this comment.
Sorry for delay.
- file
doneuses as finish-flag in backup script to detect incompleted backup (I run xtrabackup on several replicas) - file
doneis single file, it uploaded once time - xbcloud not modified,
doneuploads when xbcloud successfully finished
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I tried download backup from S3 and got error:
Some debug output for chunks processing:
Chunk name '20241105-shrinked2/done' has 23 chars (but not contains '.'), so it splits to filename '20' and idx '241105-shrinked2/done', then atoll cast idx to '241105'.
File 'done' uploaded to bucket by my backup script as finish-flag.
I append checks for right placement for
/also: it must be left side of '.'