You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/05/05 16:29:49 UTC

[GitHub] [hbase] gjacoby126 opened a new pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

gjacoby126 opened a new pull request #1655:
URL: https://github.com/apache/hbase/pull/1655


   …canOptions


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] anoopsjohn commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r422666989



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -80,4 +97,23 @@ public KeepDeletedCells getKeepDeletedCells() {
     return keepDeletedCells != null ? keepDeletedCells : scanInfo.getKeepDeletedCells();
   }
 
+  @Override
+  public int getMinVersions() {
+    return minVersions != null ? minVersions : scanInfo.getMinVersions();
+  }
+
+  @Override
+  public void setMinVersions(int minVersions) {
+    this.minVersions = minVersions;
+  }
+
+  @Override
+  public Scan getScan() {
+    try {
+      return new Scan(scan);

Review comment:
       Ok.  An ImmutableScan would have helped. Fine. As long as we dont have lets do this.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] gjacoby126 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420460681



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -34,8 +36,18 @@
 
   private KeepDeletedCells keepDeletedCells = null;
 
+  private Integer minVersions;
+
+  private final Scan scan;
+
   public CustomizedScanInfoBuilder(ScanInfo scanInfo) {
     this.scanInfo = scanInfo;
+    this.scan = new Scan();
+  }
+  public CustomizedScanInfoBuilder(ScanInfo scanInfo, Scan scan) throws IOException {

Review comment:
       Looks like the copy constructor for Scan has thrown IOException back since 2009, but the current code does not actually contain anything that throws it. Since I assume changing the signature is more trouble than it's worth, I just had to rethrow from the CustomizedScanInfoBuilder constructor and getScan() methods. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624191263


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m 29s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  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  |   4m 54s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 29s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 15s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 26s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m  6s |  hbase-server: The patch generated 2 new + 91 unchanged - 0 fixed = 93 total (was 91)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m  5s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 13s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 13s |  The patch does not generate ASF License warnings.  |
   |  |   |  36m 38s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 1e18e20b43cb 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3340c0024e |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624361621


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 42s |  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 :x: |  mvninstall  |   3m  1s |  root in master failed.  |
   | -1 :x: |  compile  |   0m 15s |  hbase-server in master failed.  |
   | +1 :green_heart: |  shadedjars  |   5m 10s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 12s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 57s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  5s |  the patch passed  |
   | -0 :warning: |  javac  |   1m  5s |  hbase-server generated 4 new + 0 unchanged - 0 fixed = 4 total (was 0)  |
   | +1 :green_heart: |  shadedjars  |   5m 10s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 41s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 124m 50s |  hbase-server in the patch passed.  |
   |  |   | 148m  7s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 513a89c331a6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 404849ab36 |
   | Default Java | 2020-01-14 |
   | mvninstall | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/artifact/yetus-jdk11-hadoop3-check/output/branch-mvninstall-root.txt |
   | compile | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/artifact/yetus-jdk11-hadoop3-check/output/branch-compile-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javac | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/artifact/yetus-jdk11-hadoop3-check/output/diff-compile-javac-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/testReport/ |
   | Max. process+thread count | 4315 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624321212


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 53s |  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  |   4m  9s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 18s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 22s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  0s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 21s |  hbase-server: The patch generated 3 new + 91 unchanged - 0 fixed = 94 total (was 91)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | -1 :x: |  hadoopcheck  |  10m  0s |  The patch causes 10 errors with Hadoop v3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 38s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 15s |  The patch does not generate ASF License warnings.  |
   |  |   |  29m 45s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 9fc762e33f07 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 404849ab36 |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | hadoopcheck | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/artifact/yetus-general-check/output/patch-javac-3.2.1.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624431932


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 48s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  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  |   4m 24s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 28s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 31s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 37s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 32s |  hbase-server: The patch generated 2 new + 91 unchanged - 0 fixed = 93 total (was 91)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  14m 46s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 34s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 13s |  The patch does not generate ASF License warnings.  |
   |  |   |  41m 53s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 99aea058218b 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3d96007c37 |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420489073



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
##########
@@ -685,6 +687,40 @@ public StoreFileReader postStoreFileReaderOpen(ObserverContext<RegionCoprocessor
     return reader;
   }
 
+  @Override
+  public void preStoreScannerOpen(ObserverContext<RegionCoprocessorEnvironment> ctx,
+    Store store, ScanOptions options) throws IOException {
+    if (options.getScan().getTimeRange().isAllTime()) {

Review comment:
       I think this class is only used to test whether a specific CP method has been called? What is this used for?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanInfo.java
##########
@@ -174,8 +174,13 @@ public boolean isNewVersionBehavior() {
    * Used for CP users for customizing max versions, ttl and keepDeletedCells.
    */
   ScanInfo customize(int maxVersions, long ttl, KeepDeletedCells keepDeletedCells) {

Review comment:
       Is this method still referenced? Just remove it?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -34,8 +36,18 @@
 
   private KeepDeletedCells keepDeletedCells = null;
 
+  private Integer minVersions;
+
+  private final Scan scan;
+
   public CustomizedScanInfoBuilder(ScanInfo scanInfo) {
     this.scanInfo = scanInfo;
+    this.scan = new Scan();
+  }
+  public CustomizedScanInfoBuilder(ScanInfo scanInfo, Scan scan) throws IOException {

Review comment:
       Let's just catch the IOException in the code here and then throw an AssertionError to indicate this should not happen, and we can filed another issue to remove the IOException from throws.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanOptions.java
##########
@@ -64,4 +66,10 @@ default void readAllVersions() {
   void setKeepDeletedCells(KeepDeletedCells keepDeletedCells);
 
   KeepDeletedCells getKeepDeletedCells();
+
+  int getMinVersions();
+
+  void setMinVersions(int minVersions);
+
+  Scan getScan() throws IOException;

Review comment:
       Please add a comment to say that the returned Scan object is only for reading, changing it will have no effect to the actual scan operation.
   And please modify the javadoc of this class to indicate that now we can also change min versions?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] anoopsjohn commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r422633969



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -80,4 +97,23 @@ public KeepDeletedCells getKeepDeletedCells() {
     return keepDeletedCells != null ? keepDeletedCells : scanInfo.getKeepDeletedCells();
   }
 
+  @Override
+  public int getMinVersions() {
+    return minVersions != null ? minVersions : scanInfo.getMinVersions();
+  }
+
+  @Override
+  public void setMinVersions(int minVersions) {
+    this.minVersions = minVersions;
+  }
+
+  @Override
+  public Scan getScan() {
+    try {
+      return new Scan(scan);

Review comment:
       +1..  That was something came to my mind too while reading the Jira desc. :-)

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -80,4 +97,23 @@ public KeepDeletedCells getKeepDeletedCells() {
     return keepDeletedCells != null ? keepDeletedCells : scanInfo.getKeepDeletedCells();
   }
 
+  @Override
+  public int getMinVersions() {
+    return minVersions != null ? minVersions : scanInfo.getMinVersions();
+  }
+
+  @Override
+  public void setMinVersions(int minVersions) {
+    this.minVersions = minVersions;
+  }
+
+  @Override
+  public Scan getScan() {
+    try {
+      return new Scan(scan);

Review comment:
       Already the scan object been cloned while setting in this class..  Every time with get also we need to clone?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624263040


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 43s |  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  |   5m 40s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 17s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m 32s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 57s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 57s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 27s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   7m  9s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 50s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 141m 11s |  hbase-server in the patch passed.  |
   |  |   | 175m  8s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 4388f141e282 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3340c0024e |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/testReport/ |
   | Max. process+thread count | 4217 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] gjacoby126 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420520899



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanInfo.java
##########
@@ -174,8 +174,13 @@ public boolean isNewVersionBehavior() {
    * Used for CP users for customizing max versions, ttl and keepDeletedCells.
    */
   ScanInfo customize(int maxVersions, long ttl, KeepDeletedCells keepDeletedCells) {

Review comment:
       It was used by a few tests: TestCompaction, TestDefaultCompaction, TestMajorCompaction. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624383024


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 12s |  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 57s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 34s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 57s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 57s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 25s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 34s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 194m 22s |  hbase-server in the patch passed.  |
   |  |   | 219m  5s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 884c3c5c9459 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 404849ab36 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/testReport/ |
   | Max. process+thread count | 3285 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420522949



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
##########
@@ -685,6 +687,40 @@ public StoreFileReader postStoreFileReaderOpen(ObserverContext<RegionCoprocessor
     return reader;
   }
 
+  @Override
+  public void preStoreScannerOpen(ObserverContext<RegionCoprocessorEnvironment> ctx,
+    Store store, ScanOptions options) throws IOException {
+    if (options.getScan().getTimeRange().isAllTime()) {

Review comment:
       OK, the rebuild of the ScanInfo is done in RegionCoprocessorHost, I used to thought it is done in HStore on some other places.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] gjacoby126 commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-625622291


   @Apache9 - I think I've addressed all your comments so far, but when you get a minute please let me know if there's any other changes you'd like, or if this is ready to go. 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] gjacoby126 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420370515



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -80,4 +93,19 @@ public KeepDeletedCells getKeepDeletedCells() {
     return keepDeletedCells != null ? keepDeletedCells : scanInfo.getKeepDeletedCells();
   }
 
+  @Override
+  public int getMinVersions() {
+    return minVersions != null ? minVersions : scanInfo.getMinVersions();
+  }
+
+  @Override
+  public void setMinVersions(int minVersions) {
+    this.minVersions = minVersions;
+  }
+
+  @Override
+  public Scan getScan() {
+    return scan;

Review comment:
       Scan is mostly mutable, I think. That's a good point -- the clone in the constructor will prevent changes from executing, but a caller could still mess up the state for future callers. I'll clone in the getter. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624497248


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 10s |  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 50s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 16s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 16s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 44s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   1m 14s |  hbase-server: The patch generated 2 new + 91 unchanged - 0 fixed = 93 total (was 91)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  12m 30s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 12s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 13s |  The patch does not generate ASF License warnings.  |
   |  |   |  36m  7s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 8b1612c5287b 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / a9a1b9524d |
   | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
   | Max. process+thread count | 84 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624499118


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m  2s |  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  |   4m 34s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  5s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 21s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 48s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 37s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 15s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 15s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 52s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 45s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |  10m 54s |  hbase-server in the patch failed.  |
   |  |   |  40m 38s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 57485366e50d 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / a9a1b9524d |
   | Default Java | 1.8.0_232 |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/testReport/ |
   | Max. process+thread count | 441 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624558934


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 39s |  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  |   5m  8s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 15s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 26s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 48s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 52s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 16s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 16s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 41s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 44s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 140m 34s |  hbase-server in the patch passed.  |
   |  |   | 170m 33s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux a2e309847344 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / a9a1b9524d |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/testReport/ |
   | Max. process+thread count | 4051 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/4/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] gjacoby126 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r422657613



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -80,4 +97,23 @@ public KeepDeletedCells getKeepDeletedCells() {
     return keepDeletedCells != null ? keepDeletedCells : scanInfo.getKeepDeletedCells();
   }
 
+  @Override
+  public int getMinVersions() {
+    return minVersions != null ? minVersions : scanInfo.getMinVersions();
+  }
+
+  @Override
+  public void setMinVersions(int minVersions) {
+    this.minVersions = minVersions;
+  }
+
+  @Override
+  public Scan getScan() {
+    try {
+      return new Scan(scan);

Review comment:
       @anoopsjohn  In the original draft it was only cloned in the constructor. @apurtell pointed out that that meant that one coproc override could call getScan(), call a setter on the Scan, and that a subsequent override would see the modified Scan, so he asked me to fix that. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] gjacoby126 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420527391



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanOptions.java
##########
@@ -64,4 +66,10 @@ default void readAllVersions() {
   void setKeepDeletedCells(KeepDeletedCells keepDeletedCells);
 
   KeepDeletedCells getKeepDeletedCells();
+
+  int getMinVersions();
+
+  void setMinVersions(int minVersions);
+
+  Scan getScan() throws IOException;

Review comment:
       Done




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] gjacoby126 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420368807



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -34,8 +36,18 @@
 
   private KeepDeletedCells keepDeletedCells = null;
 
+  private Integer minVersions;
+
+  private final Scan scan;
+
   public CustomizedScanInfoBuilder(ScanInfo scanInfo) {
     this.scanInfo = scanInfo;
+    this.scan = new Scan();
+  }
+  public CustomizedScanInfoBuilder(ScanInfo scanInfo, Scan scan) throws IOException {

Review comment:
       I was surprised too. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache9 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420529237



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanOptions.java
##########
@@ -64,4 +66,13 @@ default void readAllVersions() {
   void setKeepDeletedCells(KeepDeletedCells keepDeletedCells);
 
   KeepDeletedCells getKeepDeletedCells();
+
+  int getMinVersions();
+
+  void setMinVersions(int minVersions);
+
+  /**
+   * Returns a read-only copy of the Scan object

Review comment:
       The comment is a bit confusing as you are free to call the modification methods of the returned Scan object? So it is not 'read only'.
   
   Maybe better with "The returned Scan object is just a copy, modifying it will have no effect, so treat it as read-only."
   
   Of course I'm not a native English speaker, so feel free to use your own word. Just show the meaning that 'you can modify, but no effect, so please do not modify'.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] gjacoby126 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420520558



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
##########
@@ -685,6 +687,40 @@ public StoreFileReader postStoreFileReaderOpen(ObserverContext<RegionCoprocessor
     return reader;
   }
 
+  @Override
+  public void preStoreScannerOpen(ObserverContext<RegionCoprocessorEnvironment> ctx,
+    Store store, ScanOptions options) throws IOException {
+    if (options.getScan().getTimeRange().isAllTime()) {

Review comment:
       See TestRegionCoprocessorHost. It's used to test that the scanner coproc hooks are wired up correctly and can alter a ScanInfo. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] virajjasani commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r422598055



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -80,4 +97,23 @@ public KeepDeletedCells getKeepDeletedCells() {
     return keepDeletedCells != null ? keepDeletedCells : scanInfo.getKeepDeletedCells();
   }
 
+  @Override
+  public int getMinVersions() {
+    return minVersions != null ? minVersions : scanInfo.getMinVersions();
+  }
+
+  @Override
+  public void setMinVersions(int minVersions) {
+    this.minVersions = minVersions;
+  }
+
+  @Override
+  public Scan getScan() {
+    try {
+      return new Scan(scan);

Review comment:
       +1 to Immutable Scan wrapper with unmodifiable getters




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] gjacoby126 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420460856



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -80,4 +93,19 @@ public KeepDeletedCells getKeepDeletedCells() {
     return keepDeletedCells != null ? keepDeletedCells : scanInfo.getKeepDeletedCells();
   }
 
+  @Override
+  public int getMinVersions() {
+    return minVersions != null ? minVersions : scanInfo.getMinVersions();
+  }
+
+  @Override
+  public void setMinVersions(int minVersions) {
+    this.minVersions = minVersions;
+  }
+
+  @Override
+  public Scan getScan() {
+    return scan;

Review comment:
       Done.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624185562


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 41s |  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 48s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   4m 57s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 39s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 54s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 54s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 10s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 35s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   6m 52s |  hbase-server in the patch failed.  |
   |  |   |  29m 30s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 85127111c455 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3340c0024e |
   | Default Java | 1.8.0_232 |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/testReport/ |
   | Max. process+thread count | 770 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] gjacoby126 commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420561668



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanOptions.java
##########
@@ -64,4 +66,13 @@ default void readAllVersions() {
   void setKeepDeletedCells(KeepDeletedCells keepDeletedCells);
 
   KeepDeletedCells getKeepDeletedCells();
+
+  int getMinVersions();
+
+  void setMinVersions(int minVersions);
+
+  /**
+   * Returns a read-only copy of the Scan object

Review comment:
       Good point. Done. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624457085


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  4s |  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  |   4m  2s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  2s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 12s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 51s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  1s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  1s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m  8s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 39s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 125m 21s |  hbase-server in the patch passed.  |
   |  |   | 149m 40s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux b864d7a2252a 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3d96007c37 |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/testReport/ |
   | Max. process+thread count | 4052 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] apurtell commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420355465



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -80,4 +93,19 @@ public KeepDeletedCells getKeepDeletedCells() {
     return keepDeletedCells != null ? keepDeletedCells : scanInfo.getKeepDeletedCells();
   }
 
+  @Override
+  public int getMinVersions() {
+    return minVersions != null ? minVersions : scanInfo.getMinVersions();
+  }
+
+  @Override
+  public void setMinVersions(int minVersions) {
+    this.minVersions = minVersions;
+  }
+
+  @Override
+  public Scan getScan() {
+    return scan;

Review comment:
       I didn't check to see how much of Scan is immutable...




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#issuecomment-624481050


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  9s |  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 59s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 23s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 35s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 44s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 25s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 196m 31s |  hbase-server in the patch passed.  |
   |  |   | 221m 10s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.8 Server=19.03.8 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1655 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 7755a71dcafc 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 3d96007c37 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/testReport/ |
   | Max. process+thread count | 3554 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1655/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] lhofhansl commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
lhofhansl commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r421080372



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -80,4 +97,23 @@ public KeepDeletedCells getKeepDeletedCells() {
     return keepDeletedCells != null ? keepDeletedCells : scanInfo.getKeepDeletedCells();
   }
 
+  @Override
+  public int getMinVersions() {
+    return minVersions != null ? minVersions : scanInfo.getMinVersions();
+  }
+
+  @Override
+  public void setMinVersions(int minVersions) {
+    this.minVersions = minVersions;
+  }
+
+  @Override
+  public Scan getScan() {
+    try {
+      return new Scan(scan);

Review comment:
       Might be good at some point to have an unmodifiable subclass or wrapper of Scan, so that any modification would cause an error. (not in this PR, though)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] apurtell commented on a change in pull request #1655: HBASE-24321 - Add writable MinVersions and read-only Scan to coproc S…

Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #1655:
URL: https://github.com/apache/hbase/pull/1655#discussion_r420353851



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -80,4 +93,19 @@ public KeepDeletedCells getKeepDeletedCells() {
     return keepDeletedCells != null ? keepDeletedCells : scanInfo.getKeepDeletedCells();
   }
 
+  @Override
+  public int getMinVersions() {
+    return minVersions != null ? minVersions : scanInfo.getMinVersions();
+  }
+
+  @Override
+  public void setMinVersions(int minVersions) {
+    this.minVersions = minVersions;
+  }
+
+  @Override
+  public Scan getScan() {
+    return scan;

Review comment:
       Changes made on the returned scan object will be effective and visible beyond the caller. Clone and return?

##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionCoprocessorHost.java
##########
@@ -74,4 +105,73 @@ public void testLoadDuplicateCoprocessor() throws Exception {
     // Two duplicate coprocessors loaded
     assertEquals(2, host.coprocEnvironments.size());
   }
-}
\ No newline at end of file
+

Review comment:
       Nice additional tests.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CustomizedScanInfoBuilder.java
##########
@@ -34,8 +36,18 @@
 
   private KeepDeletedCells keepDeletedCells = null;
 
+  private Integer minVersions;
+
+  private final Scan scan;
+
   public CustomizedScanInfoBuilder(ScanInfo scanInfo) {
     this.scanInfo = scanInfo;
+    this.scan = new Scan();
+  }
+  public CustomizedScanInfoBuilder(ScanInfo scanInfo, Scan scan) throws IOException {

Review comment:
       The Scan constructor throws IOE now?? Not an issue for this patch, though




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org