Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
ndimiduk commented on PR #5871: URL: https://github.com/apache/hbase/pull/5871#issuecomment-2112749361 Merged to master. I'll start backporting and holler if I have any issues. Thanks for the contribution @DieterDP-ng and for the review @rmdmattingly ! -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
ndimiduk merged PR #5871: URL: https://github.com/apache/hbase/pull/5871 -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
joshelser commented on PR #5871: URL: https://github.com/apache/hbase/pull/5871#issuecomment-2108176794 👋🏼 sorry, I don't recall exactly what was being put into the incremental backups anymore. I remember holding on to all of the WAL files quickly got out of control and we talked about consolidating multiple incremental backups (lists of WALs) into hfiles, but I don't remember exactly what was implemented. -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
DieterDP-ng commented on code in PR #5871:
URL: https://github.com/apache/hbase/pull/5871#discussion_r1598573660
##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/HBackupFileSystem.java:
##
@@ -102,24 +95,6 @@ public static Path getTableBackupPath(TableName tableName,
Path backupRootPath,
return new Path(getTableBackupDir(backupRootPath.toString(), backupId,
tableName));
}
- /**
- * Given the backup root dir and the backup id, return the log file location
for an incremental
- * backup.
- * @param backupRootDir backup root directory
- * @param backupId backup id
- * @return logBackupDir: ".../user/biadmin/backup/WALs/backup_1396650096738"
- */
- public static String getLogBackupDir(String backupRootDir, String backupId) {
-return backupRootDir + Path.SEPARATOR + backupId + Path.SEPARATOR
- + HConstants.HREGION_LOGDIR_NAME;
- }
-
- public static Path getLogBackupPath(String backupRootDir, String backupId) {
-return new Path(getLogBackupDir(backupRootDir, backupId));
- }
-
- // TODO we do not keep WAL files anymore
Review Comment:
My interpretation of this TODO (introduced in HBASE-14135), is that it was
already implemented in that same commit. If that weren't the case, the TODO
message is too vague for me to understand what needs to be done, rendering it
useless anyway.
--
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: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
DieterDP-ng commented on PR #5871: URL: https://github.com/apache/hbase/pull/5871#issuecomment-2107727003 > Looks good to me. It would be good to solicit comments from whomever made the change that deprecated the old manifest logic in the first place. @DieterDP-ng do you know whom that might be? Maybe check the relevant commit logs. Please @-mention them on this ticket. I think this was done in HBASE-14135, committed by @joshelser, with @VladRodionov mentioned. -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
ndimiduk commented on PR #5871: URL: https://github.com/apache/hbase/pull/5871#issuecomment-2104294247 Looks like both modified test methods were executed and passed. -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
ndimiduk commented on code in PR #5871:
URL: https://github.com/apache/hbase/pull/5871#discussion_r1596521351
##
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/HBackupFileSystem.java:
##
@@ -102,24 +95,6 @@ public static Path getTableBackupPath(TableName tableName,
Path backupRootPath,
return new Path(getTableBackupDir(backupRootPath.toString(), backupId,
tableName));
}
- /**
- * Given the backup root dir and the backup id, return the log file location
for an incremental
- * backup.
- * @param backupRootDir backup root directory
- * @param backupId backup id
- * @return logBackupDir: ".../user/biadmin/backup/WALs/backup_1396650096738"
- */
- public static String getLogBackupDir(String backupRootDir, String backupId) {
-return backupRootDir + Path.SEPARATOR + backupId + Path.SEPARATOR
- + HConstants.HREGION_LOGDIR_NAME;
- }
-
- public static Path getLogBackupPath(String backupRootDir, String backupId) {
-return new Path(getLogBackupDir(backupRootDir, backupId));
- }
-
- // TODO we do not keep WAL files anymore
Review Comment:
Removal of this TODO is accurate?
--
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: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
Apache-HBase commented on PR #5871: URL: https://github.com/apache/hbase/pull/5871#issuecomment-2093661849 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 45s | Docker mode activated. | | -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck | ||| _ Prechecks _ | ||| _ master Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 56s | master passed | | +1 :green_heart: | compile | 0m 24s | master passed | | +1 :green_heart: | shadedjars | 6m 34s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 17s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 41s | the patch passed | | +1 :green_heart: | compile | 0m 20s | the patch passed | | +1 :green_heart: | javac | 0m 20s | the patch passed | | +1 :green_heart: | shadedjars | 6m 24s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 13s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 15m 3s | hbase-backup in the patch passed. | | | | 38m 46s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/5871 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux fdb28a80660d 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 3d66866f41 | | Default Java | Temurin-1.8.0_352-b08 | | Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/testReport/ | | Max. process+thread count | 3073 (vs. ulimit of 3) | | modules | C: hbase-backup U: hbase-backup | | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/console | | versions | git=2.34.1 maven=3.8.6 | | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org | This message was automatically generated. -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
Apache-HBase commented on PR #5871: URL: https://github.com/apache/hbase/pull/5871#issuecomment-2093651831 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 28s | Docker mode activated. | | -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck | ||| _ Prechecks _ | ||| _ master Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 3s | master passed | | +1 :green_heart: | compile | 0m 16s | master passed | | +1 :green_heart: | shadedjars | 5m 36s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 13s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 2m 46s | the patch passed | | +1 :green_heart: | compile | 0m 17s | the patch passed | | +1 :green_heart: | javac | 0m 17s | the patch passed | | +1 :green_heart: | shadedjars | 5m 38s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 12s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 11m 25s | hbase-backup in the patch passed. | | | | 30m 54s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/5871 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux e1020d02551a 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 3d66866f41 | | Default Java | Eclipse Adoptium-11.0.17+8 | | Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/testReport/ | | Max. process+thread count | 3763 (vs. ulimit of 3) | | modules | C: hbase-backup U: hbase-backup | | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/console | | versions | git=2.34.1 maven=3.8.6 | | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org | This message was automatically generated. -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
Apache-HBase commented on PR #5871: URL: https://github.com/apache/hbase/pull/5871#issuecomment-2093650993 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 36s | Docker mode activated. | | -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck | ||| _ Prechecks _ | ||| _ master Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 3s | master passed | | +1 :green_heart: | compile | 0m 20s | master passed | | +1 :green_heart: | shadedjars | 5m 20s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 16s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 2m 54s | the patch passed | | +1 :green_heart: | compile | 0m 19s | the patch passed | | +1 :green_heart: | javac | 0m 19s | the patch passed | | +1 :green_heart: | shadedjars | 5m 20s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 14s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | unit | 10m 53s | hbase-backup in the patch passed. | | | | 30m 16s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.45 ServerAPI=1.45 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/5871 | | Optional Tests | javac javadoc unit shadedjars compile | | uname | Linux f2737a129d82 5.4.0-172-generic #190-Ubuntu SMP Fri Feb 2 23:24:22 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 3d66866f41 | | Default Java | Eclipse Adoptium-17.0.10+7 | | Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/testReport/ | | Max. process+thread count | 3446 (vs. ulimit of 3) | | modules | C: hbase-backup U: hbase-backup | | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/console | | versions | git=2.34.1 maven=3.8.6 | | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org | This message was automatically generated. -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] HBASE-28502 Cleanup old backup manifest logic [hbase]
Apache-HBase commented on PR #5871: URL: https://github.com/apache/hbase/pull/5871#issuecomment-2093642616 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 25s | Docker mode activated. | ||| _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. | | +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. | | +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. | ||| _ master Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 24s | master passed | | +1 :green_heart: | compile | 0m 29s | master passed | | +1 :green_heart: | checkstyle | 0m 12s | master passed | | +1 :green_heart: | spotless | 0m 59s | branch has no errors when running spotless:check. | | +1 :green_heart: | spotbugs | 0m 32s | master passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 3m 2s | the patch passed | | +1 :green_heart: | compile | 0m 24s | the patch passed | | +1 :green_heart: | javac | 0m 24s | hbase-backup generated 0 new + 106 unchanged - 3 fixed = 106 total (was 109) | | +1 :green_heart: | checkstyle | 0m 8s | the patch passed | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | hadoopcheck | 5m 43s | Patch does not cause any errors with Hadoop 3.3.6. | | +1 :green_heart: | spotless | 0m 44s | patch has no errors when running spotless:check. | | +1 :green_heart: | spotbugs | 0m 36s | the patch passed | ||| _ Other Tests _ | | +1 :green_heart: | asflicense | 0m 10s | The patch does not generate ASF License warnings. | | | | 23m 55s | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/artifact/yetus-general-check/output/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/5871 | | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile | | uname | Linux 798f7c282493 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/hbase-personality.sh | | git revision | master / 3d66866f41 | | Default Java | Eclipse Adoptium-11.0.17+8 | | Max. process+thread count | 79 (vs. ulimit of 3) | | modules | C: hbase-backup U: hbase-backup | | Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5871/1/console | | versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 | | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org | This message was automatically generated. -- 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: issues-unsubscr...@hbase.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
