devmadhuu commented on PR #11563: URL: https://github.com/apache/ozone/pull/11563#issuecomment-6262575031
@smengcl @ArafatKhan2198 I wanted to have a review of this PR, so added myself as reviewer. So here are 2 points: 1. This PR has done a override in Recon specifically to handle partial transfer cleanup for Recon. since currently OM cleanup the complete candidate dir, we better should have either fixed it same way, but since we have made it a resume option to not transfer the already extracted sst files from tar, we have fixed in this PR by overriding the OM method and using the `.tmp` rename mechanism. This would have taken care at OM side as well and no need to maintain separate logic in Recon. 2. Also there is no guarantee that using this resume option, we are going to save some transfer cost. Lets try to understand with below example: **Example** The leader's DB has 10 SST files, 1 GB each: F1 … F10. The leader stops sending in the middle of F4. **First attempt:** **File** | **What Recon got** `F1, F2, F3` | `Complete. Unpacked and kept.` `F4` | `Only half. The untar fails here, and the PR deletes the `.tmp` file.` `F5...F10` | `Never arrived` Recon's candidate dir now holds F1, F2, F3. Retry on the next sync cycle: - Recon tells the leader: "I already have F1, F2, F3." - The leader sends F4 … F10, which is 7 GB. So resume saves 3 GB of the 10 GB. **Why I called it "a little"** 1. Only the files before the cut are saved. The cut could happen anywhere. Stopping early saves almost nothing. Stopping near the end saves most of it. On average that is about half the DB. 2. Everything after the cut is sent again. The retry is one more tar with all the missing files. 3. The saving may not last. If the leader compacts F2 into a new file before the retry, Recon's F2 no longer matches anything on the leader. The leader sends the new file, and Recon's old copy is useless. IMO, For this case, resume saves some download time, depending on where the cut happened. For the more likely case, Recon's disk filling up, it saves almost nothing and keeps the space in use. That's why I'd still go with wiping on failure for Recon as it works today. Keeping the OM and Recon logic as was before PR is recommended, so that logic doesn't drift for same OM db snapshot transfer either by OM follower or by Recon. No need to have separate overridden logic. cc: @sadanand48 @umamaheswararao -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
