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 2019/12/18 11:58:09 UTC

[GitHub] [hbase] virajjasani opened a new pull request #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

virajjasani opened a new pull request #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948
 
 
   …ocksOnWrite is enabled

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


With regards,
Apache Git Services

[GitHub] [hbase] virajjasani commented on a change in pull request #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#discussion_r359691101
 
 

 ##########
 File path: hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestCacheOnWrite.java
 ##########
 @@ -462,22 +472,34 @@ private void testCachingDataBlocksDuringCompactionInternals(boolean useTags,
       LOG.debug("compactStores() returned");
 
       boolean dataBlockCached = false;
+      boolean bloomBlockCached = false;
+      boolean indexBlockCached = false;
       for (CachedBlock block : blockCache) {
-        if (BlockType.ENCODED_DATA.equals(block.getBlockType())
-            || BlockType.DATA.equals(block.getBlockType())) {
+        if (DATA_BLOCK_TYPES.contains(block.getBlockType())) {
           dataBlockCached = true;
-          break;
+        } else if (BLOOM_BLOCK_TYPES.contains(block.getBlockType())) {
+          bloomBlockCached = true;
+        } else if (INDEX_BLOCK_TYPES.contains(block.getBlockType())) {
+          indexBlockCached = true;
         }
       }
 
       // Data blocks should be cached in instances where we are caching blocks on write. In the case
       // of testing
       // BucketCache, we cannot verify block type as it is not stored in the cache.
-      assertTrue(
-        "\nTest description: " + testDescription + "\ncacheBlocksOnCompaction: "
-            + cacheBlocksOnCompaction + "\n",
-        (cacheBlocksOnCompaction && !(blockCache instanceof BucketCache)) == dataBlockCached);
 
+      boolean cacheOnCompactAndNonBucketCache = cacheBlocksOnCompaction
+        && !(blockCache instanceof BucketCache);
+
+      String assertErrorMessage = "\nTest description: " + testDescription +
+        "\ncacheBlocksOnCompaction: "
+        + cacheBlocksOnCompaction + "\n";
+
+      assertEquals(assertErrorMessage, cacheOnCompactAndNonBucketCache, dataBlockCached);
+      if (cacheOnCompactAndNonBucketCache) {
+        assertTrue(assertErrorMessage, bloomBlockCached);
 
 Review comment:
   ```
   [INFO] 
   [INFO] -------------------------------------------------------
   [INFO]  T E S T S
   [INFO] -------------------------------------------------------
   [INFO] Running org.apache.hadoop.hbase.io.hfile.TestCacheOnWrite
   [INFO] Tests run: 72, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 94.853 s - in org.apache.hadoop.hbase.io.hfile.TestCacheOnWrite
   [INFO] 
   [INFO] Results:
   [INFO] 
   [INFO] Tests run: 72, Failures: 0, Errors: 0, Skipped: 0
   [INFO] 
   [INFO] 
   [INFO] --- maven-surefire-plugin:2.22.2:test (secondPartTestsExecution) @ hbase-server ---
   [INFO] Tests are skipped.
   [INFO] ------------------------------------------------------------------------
   [INFO] BUILD SUCCESS
   [INFO] ------------------------------------------------------------------------
   [INFO] Total time: 01:58 min
   [INFO] Finished at: 2019-12-19T10:51:45+05:30
   [INFO] ------------------------------------------------------------------------
   ```

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


With regards,
Apache Git Services

[GitHub] [hbase] virajjasani commented on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
virajjasani commented on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#issuecomment-570774780
 
 
   Thanks @anoopsjohn 
   Since this is merged, let me add addendum patch addressing your review comments. Agree on var name change and on too many INFO logs, let me add trace log may be?

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


With regards,
Apache Git Services

[GitHub] [hbase] virajjasani commented on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
virajjasani commented on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#issuecomment-569283677
 
 
   Thanks @ramkrish86 @chenxu14 
   Let me have one more QA build run

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


With regards,
Apache Git Services

[GitHub] [hbase] virajjasani closed pull request #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
virajjasani closed pull request #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948
 
 
   

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


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase removed a comment on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
Apache-HBase removed a comment on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#issuecomment-567088859
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  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.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 25s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 57s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 20s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   4m 34s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  master passed  |
   | +0 :ok: |  spotbugs  |   4m 20s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m 17s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 58s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 57s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 57s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedjars  |   4m 38s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  hadoopcheck  |  15m 54s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   4m 33s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 168m 55s |  hbase-server in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 36s |  The patch does not generate ASF License warnings.  |
   |  |   | 226m 44s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/948 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux ea4696e31378 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-948/out/precommit/personality/provided.sh |
   | git revision | master / 17e180e4ee |
   | Default Java | 1.8.0_181 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/1/testReport/ |
   | Max. process+thread count | 4688 (vs. ulimit of 10000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/1/console |
   | versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
   | 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


With regards,
Apache Git Services

[GitHub] [hbase] virajjasani edited a comment on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
virajjasani edited a comment on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#issuecomment-570774780
 
 
   Thanks @anoopsjohn 
   Since this is merged, let me add addendum patch addressing your review comments. Agree on var name change and on too many INFO logs, let me add trace log may be?
   
   cc: @ramkrish86 

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


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#issuecomment-570087910
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 39s |  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.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   7m 18s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 15s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 46s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 56s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 45s |  master passed  |
   | +0 :ok: |  spotbugs  |   4m 38s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m 35s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 59s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 55s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 20s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedjars  |   4m 35s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  hadoopcheck  |  15m 41s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   4m 22s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 150m 36s |  hbase-server in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 34s |  The patch does not generate ASF License warnings.  |
   |  |   | 212m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/8/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/948 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 0646e1f95bf5 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-948/out/precommit/personality/provided.sh |
   | git revision | master / e32dbe8ed2 |
   | Default Java | 1.8.0_181 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/8/testReport/ |
   | Max. process+thread count | 4906 (vs. ulimit of 10000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/8/console |
   | versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
   | 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


With regards,
Apache Git Services

[GitHub] [hbase] anoopsjohn commented on a change in pull request #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#discussion_r363027458
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
 ##########
 @@ -1120,9 +1120,25 @@ public StoreFileWriter createWriterInTmp(long maxKeyCount, Compression.Algorithm
     if (isCompaction) {
       // Don't cache data on write on compactions, unless specifically configured to do so
       writerCacheConf = new CacheConfig(cacheConf);
-      writerCacheConf.setCacheDataOnWrite(cacheConf.shouldCacheCompactedBlocksOnWrite());
+      final boolean shouldCacheCompactedBlocksOnWrite = cacheConf
+        .shouldCacheCompactedBlocksOnWrite();
+      // if data blocks are to be cached on write during compaction, we should
+      // forcefully cache index and bloom blocks as well
+      if (shouldCacheCompactedBlocksOnWrite) {
+        writerCacheConf.enableCacheOnWrite();
+        LOG.info("cacheCompactedBlocksOnWrite is true, hence enabled cacheOnWrite for " +
 
 Review comment:
   IMHO this will create so much INFO log each of which wont be giving much meaningful info later. No info which file being written.  At the RS level we can log once in INFO level that because of enableCacheOnCompaction is true, we make all 3 to true. But now it will be logged for each and every flush and compaction.

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


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase removed a comment on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
Apache-HBase removed a comment on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#issuecomment-567188685
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  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.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 27s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 55s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 20s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   4m 34s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  master passed  |
   | +0 :ok: |  spotbugs  |   4m 16s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m 14s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m  3s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  2s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  2s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 17s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedjars  |   4m 44s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  hadoopcheck  |  16m 52s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.  |
   | +1 :green_heart: |  javadoc  |   0m 35s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   4m 35s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 192m 14s |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 32s |  The patch does not generate ASF License warnings.  |
   |  |   | 250m 50s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.hbase.master.procedure.TestDisableTableProcedure |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/948 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 2c3a556b0e27 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-948/out/precommit/personality/provided.sh |
   | git revision | master / 17e180e4ee |
   | Default Java | 1.8.0_181 |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/2/artifact/out/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/2/testReport/ |
   | Max. process+thread count | 4520 (vs. ulimit of 10000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/2/console |
   | versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
   | 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


With regards,
Apache Git Services

[GitHub] [hbase] anoopsjohn commented on a change in pull request #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#discussion_r363027397
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
 ##########
 @@ -1120,9 +1120,25 @@ public StoreFileWriter createWriterInTmp(long maxKeyCount, Compression.Algorithm
     if (isCompaction) {
       // Don't cache data on write on compactions, unless specifically configured to do so
       writerCacheConf = new CacheConfig(cacheConf);
-      writerCacheConf.setCacheDataOnWrite(cacheConf.shouldCacheCompactedBlocksOnWrite());
+      final boolean shouldCacheCompactedBlocksOnWrite = cacheConf
 
 Review comment:
   Sorry for being late here.
   The var name here sounds like a method name. Ideally it should be a noun. 'cacheCompactedBlocksOnWrite'

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


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase removed a comment on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
Apache-HBase removed a comment on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#issuecomment-567451848
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  1s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 29s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 55s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 20s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   4m 41s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 37s |  master passed  |
   | +0 :ok: |  spotbugs  |   4m 23s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m 21s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 58s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 57s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 57s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 17s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  1s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedjars  |   4m 36s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  hadoopcheck  |  15m 44s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   4m 25s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 167m 38s |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 35s |  The patch does not generate ASF License warnings.  |
   |  |   | 224m 52s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.hbase.client.TestFromClientSideWithCoprocessor |
   |   | hadoop.hbase.regionserver.TestCacheOnWriteInSchema |
   |   | hadoop.hbase.client.TestFromClientSide |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/948 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux 0ef62e77c5e4 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-948/out/precommit/personality/provided.sh |
   | git revision | master / 270eb9886e |
   | Default Java | 1.8.0_181 |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/4/artifact/out/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/4/testReport/ |
   | Max. process+thread count | 4477 (vs. ulimit of 10000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/4/console |
   | versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
   | 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


With regards,
Apache Git Services

[GitHub] [hbase] Apache-HBase commented on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#issuecomment-569320900
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 34s |  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.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 3 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m 31s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 55s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 20s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   4m 37s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 39s |  master passed  |
   | +0 :ok: |  spotbugs  |   4m 16s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   4m 14s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 59s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 56s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 22s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedjars  |   4m 36s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  hadoopcheck  |  16m  2s |  Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.  |
   | +1 :green_heart: |  javadoc  |   0m 35s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   4m 37s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 156m 38s |  hbase-server in the patch failed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   | 214m 31s |   |
   
   
   | Reason | Tests |
   |-------:|:------|
   | Failed junit tests | hadoop.hbase.quotas.TestClusterScopeQuotaThrottle |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/6/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/948 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux c7445c1de32a 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-948/out/precommit/personality/provided.sh |
   | git revision | master / 0ba84d8e95 |
   | Default Java | 1.8.0_181 |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/6/artifact/out/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/6/testReport/ |
   | Max. process+thread count | 4386 (vs. ulimit of 10000) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-948/6/console |
   | versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
   | 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


With regards,
Apache Git Services

[GitHub] [hbase] virajjasani edited a comment on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…

Posted by GitBox <gi...@apache.org>.
virajjasani edited a comment on issue #948: HBASE-23588 : Cache index & bloom blocks on write if CacheCompactedBl…
URL: https://github.com/apache/hbase/pull/948#issuecomment-570774780
 
 
   Thanks @anoopsjohn 
   Since this is merged, let me add addendum patch addressing your review comments. Agree on var name change and on too many INFO logs.
   
   cc: @ramkrish86 

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


With regards,
Apache Git Services