Skip to content

Conversation

joswiatek
Copy link
Contributor

https://issues.apache.org/jira/browse/HBASE-26458

Note that this change is possible in branch-1 because the TTL determined from the snapshotProps is always included in the protobuf in this code:

public void snapshot(final String snapshotName, final TableName tableName,
SnapshotDescription.Type type, Map<String,Object> snapshotProps)
throws IOException, SnapshotCreationException, IllegalArgumentException {
SnapshotDescription.Builder builder = SnapshotDescription.newBuilder();
builder.setTable(tableName.getNameAsString());
builder.setName(snapshotName);
builder.setType(type);
builder.setTtl(getTtlFromSnapshotProps(snapshotProps));
snapshot(builder.build());
}

This is notably different from master and branch-2, where the TTL is only set if it is valid. If it is not set, then the default protobuf value of 0 is used, which matches NO_SNAPSHOT_TTL_SPECIFIED:

if (snapshotDesc.getTtl() != -1L &&
snapshotDesc.getTtl() < TimeUnit.MILLISECONDS.toSeconds(Long.MAX_VALUE)) {
builder.setTtl(snapshotDesc.getTtl());
}

@joswiatek joswiatek changed the title HBASE-22458 Add UNSET_SNAPSHOT_PROP and fix TTL defaulting HBASE-26458 Add UNSET_SNAPSHOT_PROP and fix TTL defaulting Nov 18, 2021
@joswiatek joswiatek changed the title HBASE-26458 Add UNSET_SNAPSHOT_PROP and fix TTL defaulting HBASE-26458 Update snapshot doc Nov 18, 2021
@joswiatek joswiatek changed the title HBASE-26458 Update snapshot doc HBASE-26458 Add UNSET_SNAPSHOT_PROP and fix TTL defaulting Nov 18, 2021
@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 5m 38s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ branch-1 Compile Tests _
+0 🆗 mvndep 2m 34s Maven dependency ordering for branch
+1 💚 mvninstall 9m 30s branch-1 passed
+1 💚 compile 2m 37s branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 compile 2m 54s branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 checkstyle 8m 56s branch-1 passed
+0 🆗 refguide 4m 55s branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚 shadedjars 4m 2s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 4m 27s branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javadoc 5m 2s branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+0 🆗 spotbugs 3m 28s Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚 findbugs 18m 41s branch-1 passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 20s Maven dependency ordering for patch
+1 💚 mvninstall 2m 40s the patch passed
+1 💚 compile 2m 54s the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javac 2m 54s the patch passed
+1 💚 compile 2m 43s the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 javac 2m 43s the patch passed
+1 💚 checkstyle 9m 13s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+0 🆗 refguide 4m 22s patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚 shadedjars 4m 3s patch has no errors when building our shaded downstream artifacts.
+1 💚 hadoopcheck 6m 29s Patch does not cause any errors with Hadoop 2.8.5 2.9.2.
+1 💚 javadoc 4m 2s the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javadoc 4m 44s the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 findbugs 17m 22s the patch passed
_ Other Tests _
-1 ❌ unit 199m 16s root in the patch failed.
+1 💚 asflicense 2m 12s The patch does not generate ASF License warnings.
336m 19s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/1/artifact/out/Dockerfile
GITHUB PR #3857
Optional Tests dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile refguide
uname Linux 0aaeee3177db 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality /home/jenkins/jenkins-agent/workspace/Base-PreCommit-GitHub-PR_PR-3857/out/precommit/personality/provided.sh
git revision branch-1 / 0d214ff
Default Java Azul Systems, Inc.-1.7.0_272-b10
Multi-JDK versions /usr/lib/jvm/zulu-8-amd64:Azul Systems, Inc.-1.8.0_262-b19 /usr/lib/jvm/zulu-7-amd64:Azul Systems, Inc.-1.7.0_272-b10
refguide https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/1/artifact/out/branch-site/book.html
refguide https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/1/artifact/out/patch-site/book.html
unit https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/1/artifact/out/patch-unit-root.txt
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/1/testReport/
Max. process+thread count 4118 (vs. ulimit of 10000)
modules C: hbase-common hbase-client hbase-server . U: .
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/1/console
versions git=2.17.1 maven=3.6.0 findbugs=3.0.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 37s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ branch-1 Compile Tests _
+0 🆗 mvndep 2m 32s Maven dependency ordering for branch
+1 💚 mvninstall 8m 37s branch-1 passed
+1 💚 compile 2m 18s branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 compile 2m 4s branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 checkstyle 7m 58s branch-1 passed
+0 🆗 refguide 4m 11s branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚 shadedjars 3m 24s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 3m 53s branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javadoc 4m 29s branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+0 🆗 spotbugs 2m 59s Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚 findbugs 16m 37s branch-1 passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 18s Maven dependency ordering for patch
+1 💚 mvninstall 2m 18s the patch passed
+1 💚 compile 2m 10s the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javac 2m 10s the patch passed
+1 💚 compile 2m 4s the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 javac 2m 4s the patch passed
+1 💚 checkstyle 7m 51s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+0 🆗 refguide 3m 9s patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚 shadedjars 3m 16s patch has no errors when building our shaded downstream artifacts.
+1 💚 hadoopcheck 5m 25s Patch does not cause any errors with Hadoop 2.8.5 2.9.2.
+1 💚 javadoc 3m 57s the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javadoc 4m 33s the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 findbugs 17m 8s the patch passed
_ Other Tests _
+1 💚 unit 193m 2s root in the patch passed.
+1 💚 asflicense 2m 8s The patch does not generate ASF License warnings.
309m 3s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/2/artifact/out/Dockerfile
GITHUB PR #3857
Optional Tests dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile refguide
uname Linux a402bcadd989 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality /home/jenkins/jenkins-agent/workspace/Base-PreCommit-GitHub-PR_PR-3857/out/precommit/personality/provided.sh
git revision branch-1 / 0d214ff
Default Java Azul Systems, Inc.-1.7.0_272-b10
Multi-JDK versions /usr/lib/jvm/zulu-8-amd64:Azul Systems, Inc.-1.8.0_262-b19 /usr/lib/jvm/zulu-7-amd64:Azul Systems, Inc.-1.7.0_272-b10
refguide https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/2/artifact/out/branch-site/book.html
refguide https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/2/artifact/out/patch-site/book.html
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/2/testReport/
Max. process+thread count 4129 (vs. ulimit of 10000)
modules C: hbase-common hbase-client hbase-server . U: .
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/2/console
versions git=2.17.1 maven=3.6.0 findbugs=3.0.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@virajjasani
Copy link
Contributor

@joswiatek I think here also we will have to follow the same approach as branch-2/master by converting -1 to 0 because proto already has 0 as default value of TTL that we don't want to change:

/**
 * Description of the snapshot to take
 */
message SnapshotDescription {
  required string name = 1;
  optional string table = 2; // not needed for delete, but checked for in taking snapshot
  optional int64 creation_time = 3 [default = 0];
  enum Type {
    DISABLED = 0;
    FLUSH = 1;
    SKIPFLUSH = 2;
  }
  optional Type type = 4 [default = FLUSH];
  optional int32 version = 5;
  optional string owner = 6;
  optional UsersAndPermissions users_and_permissions = 7;
  optional int64 ttl = 8 [default = 0];
}

And the reason for converting to 0 from client side remains same as we discussed: server RPC payload itself has 0 as default (non-specified) value, hence server side should handle 0 as not specified TTL rather than -1.

@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 38s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ branch-1 Compile Tests _
+0 🆗 mvndep 2m 34s Maven dependency ordering for branch
+1 💚 mvninstall 9m 0s branch-1 passed
+1 💚 compile 2m 26s branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 compile 2m 38s branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 checkstyle 8m 29s branch-1 passed
+0 🆗 refguide 5m 29s branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚 shadedjars 5m 5s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 4m 40s branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javadoc 4m 43s branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+0 🆗 spotbugs 4m 11s Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚 findbugs 23m 34s branch-1 passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 40s Maven dependency ordering for patch
+1 💚 mvninstall 3m 9s the patch passed
+1 💚 compile 2m 48s the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javac 2m 48s the patch passed
+1 💚 compile 2m 50s the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 javac 2m 50s the patch passed
+1 💚 checkstyle 10m 55s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+0 🆗 refguide 5m 9s patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚 shadedjars 5m 2s patch has no errors when building our shaded downstream artifacts.
+1 💚 hadoopcheck 7m 10s Patch does not cause any errors with Hadoop 2.8.5 2.9.2.
+1 💚 javadoc 5m 34s the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javadoc 6m 24s the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 findbugs 25m 24s the patch passed
_ Other Tests _
-1 ❌ unit 218m 57s root in the patch failed.
+1 💚 asflicense 2m 9s The patch does not generate ASF License warnings.
372m 50s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/3/artifact/out/Dockerfile
GITHUB PR #3857
Optional Tests dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile refguide
uname Linux a3f95186131c 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality /home/jenkins/jenkins-agent/workspace/Base-PreCommit-GitHub-PR_PR-3857/out/precommit/personality/provided.sh
git revision branch-1 / 1701f0d
Default Java Azul Systems, Inc.-1.7.0_272-b10
Multi-JDK versions /usr/lib/jvm/zulu-8-amd64:Azul Systems, Inc.-1.8.0_262-b19 /usr/lib/jvm/zulu-7-amd64:Azul Systems, Inc.-1.7.0_272-b10
refguide https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/3/artifact/out/branch-site/book.html
refguide https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/3/artifact/out/patch-site/book.html
unit https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/3/artifact/out/patch-unit-root.txt
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/3/testReport/
Max. process+thread count 4093 (vs. ulimit of 10000)
modules C: hbase-common hbase-client hbase-server . U: .
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/3/console
versions git=2.17.1 maven=3.6.0 findbugs=3.0.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

builder.setType(type);
builder.setTtl(getTtlFromSnapshotProps(snapshotProps));
long ttl = getTtlFromSnapshotProps(snapshotProps);
if (ttl != 1L && ttl < TimeUnit.MILLISECONDS.toSeconds(Long.MAX_VALUE)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be ttl != -1?

@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 7s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 1s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-0 ⚠️ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ branch-1 Compile Tests _
+0 🆗 mvndep 2m 31s Maven dependency ordering for branch
+1 💚 mvninstall 8m 20s branch-1 passed
+1 💚 compile 2m 5s branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 compile 2m 0s branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 checkstyle 7m 42s branch-1 passed
+0 🆗 refguide 4m 1s branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚 shadedjars 3m 10s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 3m 28s branch-1 passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javadoc 4m 14s branch-1 passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+0 🆗 spotbugs 2m 50s Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚 findbugs 15m 29s branch-1 passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 16s Maven dependency ordering for patch
+1 💚 mvninstall 2m 8s the patch passed
+1 💚 compile 2m 2s the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javac 2m 2s the patch passed
+1 💚 compile 2m 0s the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 javac 2m 0s the patch passed
+1 💚 checkstyle 7m 39s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+0 🆗 refguide 3m 0s patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect.
+1 💚 shadedjars 3m 4s patch has no errors when building our shaded downstream artifacts.
+1 💚 hadoopcheck 5m 4s Patch does not cause any errors with Hadoop 2.8.5 2.9.2.
+1 💚 javadoc 3m 33s the patch passed with JDK Azul Systems, Inc.-1.8.0_262-b19
+1 💚 javadoc 4m 10s the patch passed with JDK Azul Systems, Inc.-1.7.0_272-b10
+1 💚 findbugs 16m 3s the patch passed
_ Other Tests _
-1 ❌ unit 180m 34s root in the patch failed.
+1 💚 asflicense 2m 13s The patch does not generate ASF License warnings.
290m 47s
Reason Tests
Failed junit tests hadoop.hbase.mapreduce.TestSecureLoadIncrementalHFiles
hadoop.hbase.replication.TestReplicationSource
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/4/artifact/out/Dockerfile
GITHUB PR #3857
Optional Tests dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile refguide
uname Linux b2f09c5a2894 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3857/out/precommit/personality/provided.sh
git revision branch-1 / 1701f0d
Default Java Azul Systems, Inc.-1.7.0_272-b10
Multi-JDK versions /usr/lib/jvm/zulu-8-amd64:Azul Systems, Inc.-1.8.0_262-b19 /usr/lib/jvm/zulu-7-amd64:Azul Systems, Inc.-1.7.0_272-b10
refguide https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/4/artifact/out/branch-site/book.html
refguide https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/4/artifact/out/patch-site/book.html
unit https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/4/artifact/out/patch-unit-root.txt
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/4/testReport/
Max. process+thread count 4280 (vs. ulimit of 10000)
modules C: hbase-common hbase-client hbase-server . U: .
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3857/4/console
versions git=2.17.1 maven=3.6.0 findbugs=3.0.1
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@virajjasani virajjasani merged commit 6d9da13 into apache:branch-1 Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants