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/12/21 19:10:58 UTC
[GitHub] [hbase] taklwu opened a new pull request #2800: HBASE-25249 Adding HStoreContext
taklwu opened a new pull request #2800:
URL: https://github.com/apache/hbase/pull/2800
Adding HStoreContext which contains the metadata about the HStore. This
meta data can be used across the HFileWriter/Readers and other HStore
consumers without the need of passing around the complete store and
exposing its internals.
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-750019167
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 14s | 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 _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 47s | master passed |
| +1 :green_heart: | compile | 1m 25s | master passed |
| +1 :green_heart: | shadedjars | 7m 8s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 54s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 51s | the patch passed |
| +1 :green_heart: | compile | 1m 22s | the patch passed |
| +1 :green_heart: | javac | 1m 22s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 9s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 207m 41s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 16m 12s | hbase-mapreduce in the patch passed. |
| | | 253m 56s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4145f7a793f6 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dcb38f47db |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/4/testReport/ |
| Max. process+thread count | 3491 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/4/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r546965528
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStoreContext.java
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
+ */
+@InterfaceAudience.Private
+public class HStoreContext {
Review comment:
after a second look, you're right that this HStoreContext should have implemented `HeapSize`, I will change it for the next diff
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-752312627
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | 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 _ |
| +0 :ok: | mvndep | 0m 32s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 5s | master passed |
| +1 :green_heart: | compile | 2m 0s | master passed |
| +1 :green_heart: | shadedjars | 6m 37s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 25s | master passed |
| -0 :warning: | patch | 8m 37s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 0s | the patch passed |
| +1 :green_heart: | compile | 1m 58s | the patch passed |
| +1 :green_heart: | javac | 1m 58s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 35s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 24s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 41s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 130m 38s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 9m 25s | hbase-mapreduce in the patch passed. |
| | | 173m 53s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c8bc5b0d3721 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 / c96fbf0407 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/8/testReport/ |
| Max. process+thread count | 4565 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/8/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552321068
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -474,33 +502,14 @@ public long getBlockingFileCount() {
}
/* End implementation of StoreConfigInformation */
- /**
- * Returns the configured bytesPerChecksum value.
- * @param conf The configuration
- * @return The bytesPerChecksum that is set in the configuration
- */
- public static int getBytesPerChecksum(Configuration conf) {
- return conf.getInt(HConstants.BYTES_PER_CHECKSUM,
- HFile.DEFAULT_BYTES_PER_CHECKSUM);
- }
-
- /**
- * Returns the configured checksum algorithm.
- * @param conf The configuration
- * @return The checksum algorithm that is set in the configuration
- */
- public static ChecksumType getChecksumType(Configuration conf) {
- String checksumName = conf.get(HConstants.CHECKSUM_TYPE_NAME);
- if (checksumName == null) {
- return ChecksumType.getDefaultChecksumType();
- } else {
- return ChecksumType.nameToType(checksumName);
- }
- }
@Override
public ColumnFamilyDescriptor getColumnFamilyDescriptor() {
- return this.family;
+ return this.storeContext.getFamily();
+ }
+
+ public Encryption.Context getEncryptionContext() {
Review comment:
`getEncryptionContext` was used by `HMobStore` such we set it to `public` , it could be `package-private`.
`getStoreContext` was planned to add in a followup PR when used, tho you're right that if we have a `getStoreContext` then this `getEncryptionContext` could be removed
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-752324065
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 11s | 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 _ |
| +0 :ok: | mvndep | 0m 22s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 3s | master passed |
| +1 :green_heart: | compile | 1m 45s | master passed |
| +1 :green_heart: | shadedjars | 7m 11s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 16s | master passed |
| -0 :warning: | patch | 8m 54s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 51s | the patch passed |
| +1 :green_heart: | compile | 1m 46s | the patch passed |
| +1 :green_heart: | javac | 1m 46s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 7s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 14s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 32s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 203m 5s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 13m 31s | hbase-mapreduce in the patch passed. |
| | | 250m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/8/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux fa2632891eda 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / c96fbf0407 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/8/testReport/ |
| Max. process+thread count | 3251 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/8/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-756624495
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 23s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 49s | master passed |
| +1 :green_heart: | compile | 1m 40s | master passed |
| +1 :green_heart: | shadedjars | 7m 25s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 4s | master passed |
| -0 :warning: | patch | 8m 48s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 37s | the patch passed |
| +1 :green_heart: | compile | 1m 40s | the patch passed |
| +1 :green_heart: | javac | 1m 40s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 24s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 2s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 197m 23s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 14m 21s | hbase-mapreduce in the patch passed. |
| | | 245m 37s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 3f604015ea00 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2444d26890 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/testReport/ |
| Max. process+thread count | 3384 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749905442
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 13s | master passed |
| +1 :green_heart: | compile | 1m 35s | master passed |
| +1 :green_heart: | shadedjars | 6m 41s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 4s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 5s | the patch passed |
| +1 :green_heart: | compile | 1m 33s | the patch passed |
| +1 :green_heart: | javac | 1m 33s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 40s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 1s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 130m 11s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 9m 27s | hbase-mapreduce in the patch passed. |
| | | 169m 59s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux ddd6625ff7b5 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 / dcb38f47db |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/3/testReport/ |
| Max. process+thread count | 4599 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/3/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-750751712
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 55s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 21s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 7s | master passed |
| +1 :green_heart: | compile | 2m 21s | master passed |
| +1 :green_heart: | shadedjars | 7m 52s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 33s | master passed |
| -0 :warning: | patch | 9m 54s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 52s | the patch passed |
| +1 :green_heart: | compile | 2m 17s | the patch passed |
| +1 :green_heart: | javac | 2m 17s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 33s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 28s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 14s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 201m 13s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 13m 29s | hbase-mapreduce in the patch passed. |
| | | 254m 44s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/7/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 645148c87eab 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dcb38f47db |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/7/testReport/ |
| Max. process+thread count | 3498 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/7/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] saintstack commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755438682
Just a few nits....
----------------------------------------------------------------
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] saintstack commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r548213446
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private HStoreContext storeContext;
Review comment:
Ok. Stick that on the head of the context class justifying why it exists.
Where do you draw the line on what is in context and what is in Store? Thanks.
----------------------------------------------------------------
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] saintstack commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552254130
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private HStoreContext storeContext;
Review comment:
@taklwu
Sorry for late reply (Thanks for pinging).
> let's try to clarify your suggestion
I was asking a question, not making a suggestion -- smile.
One thing I notice is that you and @z-york talk of the Store 'info' or 'information'. So, would StoreInfo make more sense than StoreContext? It would align with ScanInfo (Yeah, StoreInfo looks like it should have ScanInfo since it a superset or should subsume ScanInfo).
Above you say this... "...we're trying to group those read-only reference such that we don't have to always use HStore object in the lower classes." Seems like you could use a version of this to answer my question (ScanInfo is 'Immutable information for scans over a store'... so StoreInfo could be Store immutable info?).
Back to your comments...
I was trying to figure when to pass Store and when I'd pass StoreInfo/StoreContext only.
Lets just have a StoreContext/StoreInfo. Lets NOT have a StoreWriterContext; i.e. a context for write-side only.
One sec.... let me look at the PR here.
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r548281843
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private StoreContext storeContext;
Review comment:
you're right, it could be final, will do it in next commit
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755541061
: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. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 34s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 28s | master passed |
| +1 :green_heart: | checkstyle | 1m 40s | master passed |
| +1 :green_heart: | spotbugs | 3m 3s | master passed |
| -0 :warning: | patch | 1m 2s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 1s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 16s | hbase-server: The patch generated 0 new + 42 unchanged - 1 fixed = 42 total (was 43) |
| +1 :green_heart: | checkstyle | 0m 19s | The patch passed checkstyle in hbase-mapreduce |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 20m 51s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 5s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 25s | The patch does not generate ASF License warnings. |
| | | 48m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/11/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux c956ffd7b703 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 / a414361ed9 |
| Max. process+thread count | 94 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/11/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755956024
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 1s | 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 _ |
| +0 :ok: | mvndep | 0m 29s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 37s | master passed |
| +1 :green_heart: | compile | 1m 41s | master passed |
| +1 :green_heart: | shadedjars | 7m 19s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 2s | master passed |
| -0 :warning: | patch | 8m 39s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 36s | the patch passed |
| +1 :green_heart: | compile | 1m 39s | the patch passed |
| +1 :green_heart: | javac | 1m 39s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 23s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 1s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 199m 4s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 14m 27s | hbase-mapreduce in the patch passed. |
| | | 246m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/12/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4eb261278c42 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a414361ed9 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/12/testReport/ |
| Max. process+thread count | 3122 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/12/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-750750054
: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 _ |
| +0 :ok: | mvndep | 0m 21s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 6s | master passed |
| +1 :green_heart: | compile | 1m 49s | master passed |
| +1 :green_heart: | shadedjars | 7m 10s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 14s | master passed |
| -0 :warning: | patch | 8m 53s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 52s | the patch passed |
| +1 :green_heart: | compile | 1m 44s | the patch passed |
| +1 :green_heart: | javac | 1m 44s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 10s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 34s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 203m 32s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 13m 38s | hbase-mapreduce in the patch passed. |
| | | 251m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/7/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 310044bd19f8 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dcb38f47db |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/7/testReport/ |
| Max. process+thread count | 3568 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/7/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755116771
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 31s | 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 _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 50s | master passed |
| +1 :green_heart: | compile | 2m 8s | master passed |
| +1 :green_heart: | shadedjars | 8m 12s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 25s | master passed |
| -0 :warning: | patch | 10m 8s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 23s | the patch passed |
| +1 :green_heart: | compile | 2m 8s | the patch passed |
| +1 :green_heart: | javac | 2m 8s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 22s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 34s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 59s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 226m 0s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 14m 48s | hbase-mapreduce in the patch passed. |
| | | 280m 11s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/9/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 87069277f821 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 90ff550000 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/9/testReport/ |
| Max. process+thread count | 3474 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/9/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552874873
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
##########
@@ -46,6 +46,8 @@
int PRIORITY_USER = 1;
int NO_PRIORITY = Integer.MIN_VALUE;
+ int getBlockSize();
Review comment:
this was new change in the latest commit of this PR, mainly it's used for `HMobStore` when calling `createWriterInTmp`. but let me change it using `getStoreContext().getBlockSize()` to avoid adding a new interface.
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552321478
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -347,6 +331,48 @@ protected HStore(final HRegion region, final ColumnFamilyDescriptor family,
cacheOnWriteLogged = false;
}
+ private StoreContext initializeStoreContext(ColumnFamilyDescriptor family) throws IOException {
+ return new StoreContext.Builder()
+ .withBloomType(family.getBloomFilterType())
+ .withCacheConfig(createCacheConf(family))
+ .withCellComparator(region.getCellComparator())
+ .withColumnFamilyDescriptor(family)
+ .withCompactedFilesSupplier(this::getCompactedFiles)
+ .withRegionFileSystem(region.getRegionFileSystem())
+ .withDefaultHFileContext(getDefaultHFileContext(family))
+ .withFavoredNodesSupplier(this::getFavoredNodes)
+ .withFamilyStoreDirectoryPath(region.getRegionFileSystem()
+ .getStoreDir(family.getNameAsString()))
+ .withRegionCoprocessorHost(region.getCoprocessorHost())
+ .build();
+ }
+
+ private InetSocketAddress[] getFavoredNodes() {
+ InetSocketAddress[] favoredNodes = null;
+ if (region.getRegionServerServices() != null) {
+ favoredNodes = region.getRegionServerServices().getFavoredNodesForRegion(
+ region.getRegionInfo().getEncodedName());
+ }
+ return favoredNodes;
+ }
+
+ private HFileContext getDefaultHFileContext(ColumnFamilyDescriptor family) throws IOException {
Review comment:
the `conf` and `region` used within this function was blocking it to be static, but thinking it again, having this big defaultHFileContext within StoreFileContext is strange and the original thought was to reuse reference if we can.
I will remove this `getDefaultHFileContext` and `defaultHFileContext` in `StoreContext`
----------------------------------------------------------------
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] saintstack commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552884573
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
##########
@@ -46,6 +46,8 @@
int PRIORITY_USER = 1;
int NO_PRIORITY = Integer.MIN_VALUE;
+ int getBlockSize();
Review comment:
Yes. Please. Then I'll +1 this nice PR. Thanks.
----------------------------------------------------------------
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] taklwu merged pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu merged pull request #2800:
URL: https://github.com/apache/hbase/pull/2800
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552389039
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileContext.java
##########
@@ -138,6 +138,10 @@ public boolean isCompressedOrEncrypted() {
return compressAlgo;
}
+ public void setCompression(Compression.Algorithm compressAlgo) {
+ this.compressAlgo = compressAlgo;
+ }
+
Review comment:
oops, I missed this change. and now it should have updated/removed.
----------------------------------------------------------------
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] z-york commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
z-york commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r548209948
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private HStoreContext storeContext;
Review comment:
Yeah the idea is that HStore has way more scope than most consumers of HStore actually need (1. The information about the store and 2. the functions to mutate things - A lot of callers just care about the info, not the actual functions) so it made sense to separate out the info to a POJO to reduce the scope of what is being exposed.
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749862873
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | 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 _ |
| +0 :ok: | mvndep | 0m 29s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 25s | master passed |
| +1 :green_heart: | checkstyle | 1m 21s | master passed |
| +1 :green_heart: | spotbugs | 2m 37s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 31s | the patch passed |
| -0 :warning: | checkstyle | 1m 4s | hbase-server: The patch generated 2 new + 42 unchanged - 1 fixed = 44 total (was 43) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 16m 58s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 59s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 25s | The patch does not generate ASF License warnings. |
| | | 41m 16s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 73d40fc3e16b 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 / dcb38f47db |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 94 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/3/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] saintstack commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552381911
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -474,33 +502,14 @@ public long getBlockingFileCount() {
}
/* End implementation of StoreConfigInformation */
- /**
- * Returns the configured bytesPerChecksum value.
- * @param conf The configuration
- * @return The bytesPerChecksum that is set in the configuration
- */
- public static int getBytesPerChecksum(Configuration conf) {
- return conf.getInt(HConstants.BYTES_PER_CHECKSUM,
- HFile.DEFAULT_BYTES_PER_CHECKSUM);
- }
-
- /**
- * Returns the configured checksum algorithm.
- * @param conf The configuration
- * @return The checksum algorithm that is set in the configuration
- */
- public static ChecksumType getChecksumType(Configuration conf) {
- String checksumName = conf.get(HConstants.CHECKSUM_TYPE_NAME);
- if (checksumName == null) {
- return ChecksumType.getDefaultChecksumType();
- } else {
- return ChecksumType.nameToType(checksumName);
- }
- }
@Override
public ColumnFamilyDescriptor getColumnFamilyDescriptor() {
- return this.family;
+ return this.storeContext.getFamily();
+ }
+
+ public Encryption.Context getEncryptionContext() {
Review comment:
Package private sounds good if only used locally.
----------------------------------------------------------------
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] saintstack commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552821049
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -308,7 +294,7 @@ protected HStore(final HRegion region, final ColumnFamilyDescriptor family,
this.compactionCheckMultiplier = DEFAULT_COMPACTCHECKER_INTERVAL_MULTIPLIER;
}
- this.storeEngine = createStoreEngine(this, this.conf, this.comparator);
+ this.storeEngine = createStoreEngine(this, this.conf, getComparator());
Review comment:
ditto
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -268,34 +251,37 @@ protected HStore(final HRegion region, final ColumnFamilyDescriptor family,
.addBytesMap(region.getTableDescriptor().getValues())
.addStringMap(family.getConfiguration())
.addBytesMap(family.getValues());
- this.blocksize = family.getBlocksize();
+
+ this.region = region;
+ this.storeContext = initializeStoreContext(family);
+
+ // Assemble the store's home directory and Ensure it exists.
+ getRegionFileSystem().createStoreDir(family.getNameAsString());
// set block storage policy for store directory
String policyName = family.getStoragePolicy();
if (null == policyName) {
policyName = this.conf.get(BLOCK_STORAGE_POLICY_KEY, DEFAULT_BLOCK_STORAGE_POLICY);
}
- this.fs.setStoragePolicy(family.getNameAsString(), policyName.trim());
+ getRegionFileSystem().setStoragePolicy(getColumnFamilyName(), policyName.trim());
Review comment:
You were going to change these back?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
##########
@@ -46,6 +46,8 @@
int PRIORITY_USER = 1;
int NO_PRIORITY = Integer.MIN_VALUE;
+ int getBlockSize();
Review comment:
Is this necessary? Its available on the ColumnFamilyDescriptor. Do we need to add new method on Store?
Was this change always here or did it just show up in recent amendments to PRs (I don't remember seeing it before but probably just me).
Thanks.
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552321526
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -254,48 +244,47 @@
protected HStore(final HRegion region, final ColumnFamilyDescriptor family,
final Configuration confParam, boolean warmup) throws IOException {
- this.fs = region.getRegionFileSystem();
-
- // Assemble the store's home directory and Ensure it exists.
- fs.createStoreDir(family.getNameAsString());
- this.region = region;
- this.family = family;
// 'conf' renamed to 'confParam' b/c we use this.conf in the constructor
// CompoundConfiguration will look for keys in reverse order of addition, so we'd
// add global config first, then table and cf overrides, then cf metadata.
this.conf = new CompoundConfiguration()
- .add(confParam)
- .addBytesMap(region.getTableDescriptor().getValues())
- .addStringMap(family.getConfiguration())
- .addBytesMap(family.getValues());
- this.blocksize = family.getBlocksize();
+ .add(confParam)
+ .addBytesMap(region.getTableDescriptor().getValues())
+ .addStringMap(family.getConfiguration())
+ .addBytesMap(family.getValues());
+
+ this.region = region;
+ this.storeContext = initializeStoreContext(family);
+
+ // Assemble the store's home directory and Ensure it exists.
+ getRegionFileSystem().createStoreDir(getColumnFamilyName());
+
+ this.blocksize = getColumnFamilyDescriptor().getBlocksize();
// set block storage policy for store directory
- String policyName = family.getStoragePolicy();
+ String policyName = getColumnFamilyDescriptor().getStoragePolicy();
Review comment:
fixed.
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552321656
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileContext.java
##########
@@ -138,6 +138,10 @@ public boolean isCompressedOrEncrypted() {
return compressAlgo;
}
+ public void setCompression(Compression.Algorithm compressAlgo) {
+ this.compressAlgo = compressAlgo;
+ }
+
Review comment:
fixed, and reverted back to builder pattern
----------------------------------------------------------------
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] saintstack commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r547688417
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreUtils.java
##########
@@ -136,4 +140,26 @@ public static OptionalLong getMaxSequenceIdInList(Collection<HStoreFile> sfs) {
return largestFile.isPresent() ? StoreUtils.getFileSplitPoint(largestFile.get(), comparator)
: Optional.empty();
}
+
+ /**
+ * Returns the configured checksum algorithm.
+ * @param conf The configuration
+ * @return The checksum algorithm that is set in the configuration
+ */
+ public static ChecksumType getChecksumType(Configuration conf) {
+ String checksumName = conf.get(HConstants.CHECKSUM_TYPE_NAME);
Review comment:
You don't want to do get
return ChecksymType.nameToType(conf.get(HConstants.CHECKSUM_TYPE_NAME, DEFAULT_WHATEVER_IT_IS));
Is this a candidate for StoreContext? (Perhaps if called frequently). Perhaps StoreContext is not available where this is used?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private HStoreContext storeContext;
Review comment:
Yeah, just trying to understand why. You want to swap out the HStore implementation or something?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private HStoreContext storeContext;
Review comment:
Why introduce an StoreContext? Isn't that what a Store is? Store spans files.
(We need the 'H' in HStoreContext?)
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r554147880
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreContext.java
##########
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.crypto.Encryption;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.util.ClassSize;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This carries the immutable information and references on some of the meta data about the HStore.
+ * This meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
+ */
+@InterfaceAudience.Private
+public final class StoreContext implements HeapSize {
+ public static final long FIXED_OVERHEAD = ClassSize.estimateBase(HStore.class, false);
+
+ private final int blockSize;
+ private final Encryption.Context encryptionContext;
+ private final CacheConfig cacheConf;
+ private final HRegionFileSystem regionFileSystem;
+ private final CellComparator comparator;
+ private final BloomType bloomFilterType;
+ private final Supplier<Collection<HStoreFile>> compactedFilesSupplier;
+ private final Supplier<InetSocketAddress[]> favoredNodesSupplier;
+ private final ColumnFamilyDescriptor family;
+ private final Path familyStoreDirectoryPath;
+ private final RegionCoprocessorHost coprocessorHost;
+
+ private StoreContext(Builder builder) {
+ this.blockSize = builder.blockSize;
+ this.encryptionContext = builder.encryptionContext;
+ this.cacheConf = builder.cacheConf;
+ this.regionFileSystem = builder.regionFileSystem;
+ this.comparator = builder.comparator;
+ this.bloomFilterType = builder.bloomFilterType;
+ this.compactedFilesSupplier = builder.compactedFilesSupplier;
+ this.favoredNodesSupplier = builder.favoredNodesSupplier;
+ this.family = builder.family;
+ this.familyStoreDirectoryPath = builder.familyStoreDirectoryPath;
+ this.coprocessorHost = builder.coprocessorHost;
+ }
+
+ public int getBlockSize() {
+ return blockSize;
+ }
+
+ public Encryption.Context getEncryptionContext() {
+ return encryptionContext;
+ }
+
+ public CacheConfig getCacheConf() {
+ return cacheConf;
+ }
+
+ public HRegionFileSystem getRegionFileSystem() {
+ return regionFileSystem;
+ }
+
+ public CellComparator getComparator() {
+ return comparator;
+ }
+
+ public BloomType getBloomFilterType() {
+ return bloomFilterType;
+ }
+
+ public Supplier<Collection<HStoreFile>> getCompactedFilesSupplier() {
+ return compactedFilesSupplier;
+ }
+
+ public InetSocketAddress[] getFavoredNodes() {
+ return favoredNodesSupplier.get();
+ }
+
+ public ColumnFamilyDescriptor getFamily() {
+ return family;
+ }
+
+ public Path getFamilyStoreDirectoryPath() {
+ return familyStoreDirectoryPath;
+ }
+
+ public RegionCoprocessorHost getCoprocessorHost() {
+ return coprocessorHost;
+ }
+
+ public static Builder getBuilder() {
+ return new Builder();
+ }
+
+ @Override
+ public long heapSize() {
+ return FIXED_OVERHEAD;
+ }
Review comment:
@apurtell sorry for pinging again, I will merge this evening if I don't hear from you
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552320678
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreContext.java
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.hadoop.hbase.util.ClassSize;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
Review comment:
thanks for sharing a list of *Info and *Context, I browsed few of them and seems Context should be still okie. so let's keep it with `StoreContext` and made this change simple, and I add the `immutable` keyword in the block of the class comment.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1206,53 +1218,34 @@ public StoreFileWriter createWriterInTmp(long maxKeyCount, Compression.Algorithm
}
}
}
- InetSocketAddress[] favoredNodes = null;
- if (region.getRegionServerServices() != null) {
- favoredNodes = region.getRegionServerServices().getFavoredNodesForRegion(
- region.getRegionInfo().getEncodedName());
- }
- HFileContext hFileContext = createFileContext(compression, includeMVCCReadpoint, includesTag,
- cryptoContext);
- Path familyTempDir = new Path(fs.getTempDir(), family.getNameAsString());
- StoreFileWriter.Builder builder = new StoreFileWriter.Builder(conf, writerCacheConf,
- this.getFileSystem())
- .withOutputDir(familyTempDir)
- .withBloomType(family.getBloomFilterType())
- .withMaxKeyCount(maxKeyCount)
- .withFavoredNodes(favoredNodes)
- .withFileContext(hFileContext)
- .withShouldDropCacheBehind(shouldDropBehind)
- .withCompactedFilesSupplier(this::getCompactedFiles)
- .withFileStoragePolicy(fileStoragePolicy);
+ HFileContext hFileContext = createFileContext(compression, includeMVCCReadpoint, includesTag);
+ Path familyTempDir = new Path(getRegionFileSystem().getTempDir(), getColumnFamilyName());
+ StoreFileWriter.Builder builder =
+ new StoreFileWriter.Builder(conf, writerCacheConf, getFileSystem())
+ .withOutputDir(familyTempDir)
+ .withBloomType(storeContext.getBloomFilterType())
+ .withMaxKeyCount(maxKeyCount)
+ .withFavoredNodes(storeContext.getFavoredNodes())
+ .withFileContext(hFileContext)
+ .withShouldDropCacheBehind(shouldDropBehind)
+ .withCompactedFilesSupplier(storeContext.getCompactedFilesSupplier())
+ .withFileStoragePolicy(fileStoragePolicy);
return builder.build();
}
private HFileContext createFileContext(Compression.Algorithm compression,
- boolean includeMVCCReadpoint, boolean includesTag, Encryption.Context cryptoContext) {
+ boolean includeMVCCReadpoint, boolean includesTag) {
if (compression == null) {
compression = HFile.DEFAULT_COMPRESSION_ALGORITHM;
}
- HFileContext hFileContext = new HFileContextBuilder()
- .withIncludesMvcc(includeMVCCReadpoint)
- .withIncludesTags(includesTag)
- .withCompression(compression)
- .withCompressTags(family.isCompressTags())
- .withChecksumType(checksumType)
- .withBytesPerCheckSum(bytesPerChecksum)
- .withBlockSize(blocksize)
- .withHBaseCheckSum(true)
- .withDataBlockEncoding(family.getDataBlockEncoding())
- .withEncryptionContext(cryptoContext)
- .withCreateTime(EnvironmentEdgeManager.currentTime())
- .withColumnFamily(family.getName())
- .withTableName(region.getTableDescriptor()
- .getTableName().getName())
- .withCellComparator(this.comparator)
- .build();
- return hFileContext;
+ HFileContext fileContext = storeContext.getDefaultFileContext();
+ fileContext.setIncludesMvcc(includeMVCCReadpoint);
+ fileContext.setIncludesTags(includesTag);
+ fileContext.setCompression(compression);
+ fileContext.setFileCreateTime(EnvironmentEdgeManager.currentTime());
Review comment:
it was trying to reuse the `defaultFileContext` built with `initializeStoreContext`, but after rethinking on your comment, I'm going to remove the `defaultFileContext` and go back with the original builder pattern.
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749928195
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 14s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 21s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 7s | master passed |
| +1 :green_heart: | compile | 1m 24s | master passed |
| +1 :green_heart: | shadedjars | 7m 5s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 56s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 54s | the patch passed |
| +1 :green_heart: | compile | 1m 23s | the patch passed |
| +1 :green_heart: | javac | 1m 23s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 11s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 54s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 203m 44s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 13m 38s | hbase-mapreduce in the patch passed. |
| | | 247m 59s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1427b1f410bb 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dcb38f47db |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/3/testReport/ |
| Max. process+thread count | 3103 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/3/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755044679
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 43s | 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 _ |
| +0 :ok: | mvndep | 0m 30s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 11s | master passed |
| +1 :green_heart: | checkstyle | 2m 3s | master passed |
| +1 :green_heart: | spotbugs | 4m 11s | master passed |
| -0 :warning: | patch | 1m 14s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 19s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 29s | The patch passed checkstyle in hbase-common |
| +1 :green_heart: | checkstyle | 1m 21s | hbase-server: The patch generated 0 new + 46 unchanged - 1 fixed = 46 total (was 47) |
| +1 :green_heart: | checkstyle | 0m 22s | The patch passed checkstyle in hbase-mapreduce |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 23m 9s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 4m 3s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 46s | The patch does not generate ASF License warnings. |
| | | 56m 0s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/9/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 3dcbb8e8d0e1 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 / 90ff550000 |
| Max. process+thread count | 94 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/9/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-750707146
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 55s | 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 _ |
| +0 :ok: | mvndep | 0m 22s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 19s | master passed |
| +1 :green_heart: | checkstyle | 2m 1s | master passed |
| +1 :green_heart: | spotbugs | 3m 59s | master passed |
| -0 :warning: | patch | 2m 39s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 51s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 25s | The patch passed checkstyle in hbase-common |
| +1 :green_heart: | checkstyle | 1m 16s | hbase-server: The patch generated 0 new + 42 unchanged - 1 fixed = 42 total (was 43) |
| +1 :green_heart: | checkstyle | 0m 17s | The patch passed checkstyle in hbase-mapreduce |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 20m 2s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 5m 12s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 40s | The patch does not generate ASF License warnings. |
| | | 54m 28s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/7/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux a7502c5711e0 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dcb38f47db |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/7/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749986720
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | 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 _ |
| +0 :ok: | mvndep | 0m 25s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 4s | master passed |
| +1 :green_heart: | compile | 1m 34s | master passed |
| +1 :green_heart: | shadedjars | 6m 40s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 2s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 5s | the patch passed |
| +1 :green_heart: | compile | 1m 32s | the patch passed |
| +1 :green_heart: | javac | 1m 32s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 45s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 1s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 130m 5s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 9m 27s | hbase-mapreduce in the patch passed. |
| | | 169m 42s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 98ff9ee9b534 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 / dcb38f47db |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/4/testReport/ |
| Max. process+thread count | 4603 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/4/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755761858
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 0s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 20s | master passed |
| +1 :green_heart: | compile | 1m 23s | master passed |
| +1 :green_heart: | shadedjars | 7m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 56s | master passed |
| -0 :warning: | patch | 8m 25s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 38s | the patch passed |
| +1 :green_heart: | compile | 2m 3s | the patch passed |
| +1 :green_heart: | javac | 2m 3s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 16s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 54s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 210m 15s | hbase-server in the patch failed. |
| +1 :green_heart: | unit | 13m 30s | hbase-mapreduce in the patch passed. |
| | | 256m 54s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/11/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux ec977a29957b 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a414361ed9 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/11/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/11/testReport/ |
| Max. process+thread count | 3735 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/11/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749176307
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | 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 _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 16s | master passed |
| +1 :green_heart: | compile | 1m 34s | master passed |
| +1 :green_heart: | shadedjars | 6m 42s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 4s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 0s | the patch passed |
| +1 :green_heart: | compile | 1m 49s | the patch passed |
| +1 :green_heart: | javac | 1m 49s | the patch passed |
| -1 :x: | shadedjars | 6m 34s | patch has 10 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 7s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 7m 45s | hbase-server in the patch failed. |
| -1 :x: | unit | 26m 20s | hbase-mapreduce in the patch failed. |
| | | 65m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux cac64143746e 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 / dcb38f47db |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-mapreduce.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/testReport/ |
| Max. process+thread count | 2692 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] z-york commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
z-york commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r548200446
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private StoreContext storeContext;
Review comment:
Should this be final? I wouldn't expect you would need to change StoreContext (the reference) after it has been initialized
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1236,18 +1248,19 @@ private HFileContext createFileContext(Compression.Algorithm compression,
.withIncludesMvcc(includeMVCCReadpoint)
.withIncludesTags(includesTag)
.withCompression(compression)
- .withCompressTags(family.isCompressTags())
- .withChecksumType(checksumType)
- .withBytesPerCheckSum(bytesPerChecksum)
+ .withCompressTags(getColumnFamilyDescriptor().isCompressTags())
Review comment:
Does it simplify things to be able to pass StoreContext directly to the builders?
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r548286395
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1236,18 +1248,19 @@ private HFileContext createFileContext(Compression.Algorithm compression,
.withIncludesMvcc(includeMVCCReadpoint)
.withIncludesTags(includesTag)
.withCompression(compression)
- .withCompressTags(family.isCompressTags())
- .withChecksumType(checksumType)
- .withBytesPerCheckSum(bytesPerChecksum)
+ .withCompressTags(getColumnFamilyDescriptor().isCompressTags())
Review comment:
I'd like to keep the current way that passing required fields to the creation of `HFileContext`. Mainly `HFileContext` is also a member of the `StoreContext` which if we pass into `StoreContext` to the creation of `HFileContext`, it will be a loop and is very strange.
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r546951790
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -474,33 +501,14 @@ public long getBlockingFileCount() {
}
/* End implementation of StoreConfigInformation */
- /**
- * Returns the configured bytesPerChecksum value.
- * @param conf The configuration
- * @return The bytesPerChecksum that is set in the configuration
- */
- public static int getBytesPerChecksum(Configuration conf) {
- return conf.getInt(HConstants.BYTES_PER_CHECKSUM,
- HFile.DEFAULT_BYTES_PER_CHECKSUM);
- }
-
- /**
- * Returns the configured checksum algorithm.
- * @param conf The configuration
- * @return The checksum algorithm that is set in the configuration
- */
- public static ChecksumType getChecksumType(Configuration conf) {
- String checksumName = conf.get(HConstants.CHECKSUM_TYPE_NAME);
- if (checksumName == null) {
- return ChecksumType.getDefaultChecksumType();
- } else {
- return ChecksumType.nameToType(checksumName);
- }
- }
@Override
public ColumnFamilyDescriptor getColumnFamilyDescriptor() {
- return this.family;
+ return this.storeContext.getFamily();
+ }
+
+ public Encryption.Context getCryptoContext() {
Review comment:
sure, and thanks for pointing it out
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -2559,7 +2572,7 @@ public boolean needsCompaction() {
* @return cache configuration for this Store.
*/
public CacheConfig getCacheConfig() {
- return this.cacheConf;
+ return storeContext.getCacheConf();
}
public static final long FIXED_OVERHEAD = ClassSize.estimateBase(HStore.class, false);
Review comment:
TestHeapSize still passed when I checked it locally.
from what I understood from the `FIXED_OVERHEAD` were being calculated by `ClassSize.estimateBase`, it will automatically calculate the our newly added fields and reference included the change of `HStoreContext storeContext`.
So for the `DEEP_OVERHEAD` that takes new calculated `FIXED_OVERHEAD` to come up the heap size, it should be done already. Or did I miss something here?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStoreContext.java
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
+ */
+@InterfaceAudience.Private
+public class HStoreContext {
Review comment:
as I mentioned above about `FIXED_OVERHEAD` were being calculated by `ClassSize.estimateBase`, it should be already covered
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-756598611
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | 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 _ |
| +0 :ok: | mvndep | 0m 27s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 26s | master passed |
| +1 :green_heart: | compile | 1m 21s | master passed |
| +1 :green_heart: | shadedjars | 6m 35s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | master passed |
| -0 :warning: | patch | 7m 52s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 27s | the patch passed |
| +1 :green_heart: | compile | 1m 23s | the patch passed |
| +1 :green_heart: | javac | 1m 23s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 35s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 144m 8s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 11m 38s | hbase-mapreduce in the patch passed. |
| | | 183m 45s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 65477f3eada4 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 / 2444d26890 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/testReport/ |
| Max. process+thread count | 4544 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu merged pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu merged pull request #2800:
URL: https://github.com/apache/hbase/pull/2800
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749940604
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | 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 _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 27s | master passed |
| +1 :green_heart: | checkstyle | 1m 21s | master passed |
| +1 :green_heart: | spotbugs | 2m 40s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 26s | the patch passed |
| -0 :warning: | checkstyle | 1m 4s | hbase-server: The patch generated 1 new + 42 unchanged - 1 fixed = 43 total (was 43) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 17m 8s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 0s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 25s | The patch does not generate ASF License warnings. |
| | | 41m 10s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux b47d0978cd85 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 / dcb38f47db |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 94 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/4/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] saintstack commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552382472
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileContext.java
##########
@@ -138,6 +138,10 @@ public boolean isCompressedOrEncrypted() {
return compressAlgo;
}
+ public void setCompression(Compression.Algorithm compressAlgo) {
+ this.compressAlgo = compressAlgo;
+ }
+
Review comment:
You haven't uploaded new PR so will leave this as unresovled for now.
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552323137
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private HStoreContext storeContext;
Review comment:
thanks, I should have provide a followup in the latest diff and proposed to keep with the `StoreContext` naming.
marked this conversation as resolved.
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749174556
:broken_heart: **-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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 40s | master passed |
| +1 :green_heart: | compile | 1m 25s | master passed |
| +1 :green_heart: | shadedjars | 6m 41s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 57s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 34s | the patch passed |
| +1 :green_heart: | compile | 1m 21s | the patch passed |
| +1 :green_heart: | javac | 1m 21s | the patch passed |
| -1 :x: | shadedjars | 5m 11s | patch has 10 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 7m 12s | hbase-server in the patch failed. |
| -1 :x: | unit | 26m 34s | hbase-mapreduce in the patch failed. |
| | | 60m 28s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 7795aba2ba8d 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 / dcb38f47db |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-jdk8-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-mapreduce.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/testReport/ |
| Max. process+thread count | 2358 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-756552901
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 37s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 2s | master passed |
| +1 :green_heart: | checkstyle | 1m 49s | master passed |
| +1 :green_heart: | spotbugs | 3m 33s | master passed |
| -0 :warning: | patch | 2m 50s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 30s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 26s | hbase-server: The patch generated 0 new + 42 unchanged - 1 fixed = 42 total (was 43) |
| +1 :green_heart: | checkstyle | 0m 21s | The patch passed checkstyle in hbase-mapreduce |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 22m 46s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 4m 0s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 27s | The patch does not generate ASF License warnings. |
| | | 55m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 7d1b92544cb1 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2444d26890 |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755726798
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 49s | master passed |
| +1 :green_heart: | compile | 1m 47s | master passed |
| +1 :green_heart: | shadedjars | 8m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 11s | master passed |
| -0 :warning: | patch | 9m 48s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 56s | the patch passed |
| +1 :green_heart: | compile | 1m 55s | the patch passed |
| +1 :green_heart: | javac | 1m 55s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 24s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 15s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 139m 10s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 10m 29s | hbase-mapreduce in the patch passed. |
| | | 185m 42s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/11/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 8eaafe54df56 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a414361ed9 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/11/testReport/ |
| Max. process+thread count | 4067 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/11/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r551656699
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStoreContext.java
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
+ */
+@InterfaceAudience.Private
+public class HStoreContext {
Review comment:
fixed and implemented `heapSize()`
----------------------------------------------------------------
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] z-york commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
z-york commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r549513396
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreContext.java
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.hadoop.hbase.util.ClassSize;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
+ */
+@InterfaceAudience.Private
+public final class StoreContext implements HeapSize {
+ public static final long FIXED_OVERHEAD = ClassSize.estimateBase(HStore.class, false);
+
+ private final HFileContext defaultFileContext;
+ private final CacheConfig cacheConf;
+ private final HRegionFileSystem regionFileSystem;
+ private final CellComparator comparator;
+ private final BloomType bloomFilterType;
+ private final Supplier<Collection<HStoreFile>> compactedFilesSupplier;
+ private final Supplier<InetSocketAddress[]> favoredNodesSupplier;
+ private final ColumnFamilyDescriptor family;
+ private final Path familyStoreDirectoryPath;
+ private final RegionCoprocessorHost coprocessorHost;
+
+ private StoreContext(Builder builder) {
+ this.defaultFileContext = builder.defaultFileContext;
+ this.cacheConf = builder.cacheConf;
+ this.regionFileSystem = builder.regionFileSystem;
+ this.comparator = builder.comparator;
+ this.bloomFilterType = builder.bloomFilterType;
+ this.compactedFilesSupplier = builder.compactedFilesSupplier;
+ this.favoredNodesSupplier = builder.favoredNodesSupplier;
+ this.family = builder.family;
+ this.familyStoreDirectoryPath = builder.familyStoreDirectoryPath;
+ this.coprocessorHost = builder.coprocessorHost;
+ }
+
+ public HFileContext getDefaultFileContext() {
+ return defaultFileContext;
+ }
+
+ public CacheConfig getCacheConf() {
+ return cacheConf;
+ }
+
+ public HRegionFileSystem getRegionFileSystem() {
+ return regionFileSystem;
+ }
+
+ public CellComparator getComparator() {
+ return comparator;
+ }
+
+ public BloomType getBloomFilterType() {
+ return bloomFilterType;
+ }
+
+ public Supplier<Collection<HStoreFile>> getCompactedFilesSupplier() {
+ return compactedFilesSupplier;
+ }
+
+ public Supplier<InetSocketAddress[]> getFavoredNodesSupplier() {
+ return favoredNodesSupplier;
+ }
Review comment:
Does it make sense to return the supplier or just a straight getter where internal to this method it calls and returns the result of the .get()?
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r549894991
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreContext.java
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.hadoop.hbase.util.ClassSize;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
+ */
+@InterfaceAudience.Private
+public final class StoreContext implements HeapSize {
+ public static final long FIXED_OVERHEAD = ClassSize.estimateBase(HStore.class, false);
+
+ private final HFileContext defaultFileContext;
+ private final CacheConfig cacheConf;
+ private final HRegionFileSystem regionFileSystem;
+ private final CellComparator comparator;
+ private final BloomType bloomFilterType;
+ private final Supplier<Collection<HStoreFile>> compactedFilesSupplier;
+ private final Supplier<InetSocketAddress[]> favoredNodesSupplier;
+ private final ColumnFamilyDescriptor family;
+ private final Path familyStoreDirectoryPath;
+ private final RegionCoprocessorHost coprocessorHost;
+
+ private StoreContext(Builder builder) {
+ this.defaultFileContext = builder.defaultFileContext;
+ this.cacheConf = builder.cacheConf;
+ this.regionFileSystem = builder.regionFileSystem;
+ this.comparator = builder.comparator;
+ this.bloomFilterType = builder.bloomFilterType;
+ this.compactedFilesSupplier = builder.compactedFilesSupplier;
+ this.favoredNodesSupplier = builder.favoredNodesSupplier;
+ this.family = builder.family;
+ this.familyStoreDirectoryPath = builder.familyStoreDirectoryPath;
+ this.coprocessorHost = builder.coprocessorHost;
+ }
+
+ public HFileContext getDefaultFileContext() {
+ return defaultFileContext;
+ }
+
+ public CacheConfig getCacheConf() {
+ return cacheConf;
+ }
+
+ public HRegionFileSystem getRegionFileSystem() {
+ return regionFileSystem;
+ }
+
+ public CellComparator getComparator() {
+ return comparator;
+ }
+
+ public BloomType getBloomFilterType() {
+ return bloomFilterType;
+ }
+
+ public Supplier<Collection<HStoreFile>> getCompactedFilesSupplier() {
+ return compactedFilesSupplier;
+ }
+
+ public Supplier<InetSocketAddress[]> getFavoredNodesSupplier() {
+ return favoredNodesSupplier;
+ }
Review comment:
For the `getCompactedFilesSupplier()`, we should keep it as using supplier because the actual values at that time will be used when `StoreFileWriter.java#appendMetadata` is being called for writing `COMPACTION_EVENT_KEY`. unless we change the builder of `StoreFileWriter`, I would propose to keep the supplier like this.
for the `getFavoredNodesSupplier()`, I think you may be right that `withFavoredNodes` could be using the values directly per writer creation, will update in next change.
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755140184
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 38s | 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 _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 59s | master passed |
| +1 :green_heart: | checkstyle | 1m 48s | master passed |
| +1 :green_heart: | spotbugs | 3m 20s | master passed |
| -0 :warning: | patch | 2m 40s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 42s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 27s | hbase-server: The patch generated 0 new + 46 unchanged - 1 fixed = 46 total (was 47) |
| +1 :green_heart: | checkstyle | 0m 21s | The patch passed checkstyle in hbase-mapreduce |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 22m 47s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 4m 18s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 24s | The patch does not generate ASF License warnings. |
| | | 55m 51s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/10/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux fe61ee2689c4 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a5eb8f1f70 |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/10/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749850298
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | 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 _ |
| +0 :ok: | mvndep | 0m 26s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 59s | master passed |
| +1 :green_heart: | compile | 1m 34s | master passed |
| +1 :green_heart: | shadedjars | 6m 38s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 0s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 8s | the patch passed |
| +1 :green_heart: | compile | 1m 39s | the patch passed |
| +1 :green_heart: | javac | 1m 39s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 56s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 14s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 142m 2s | hbase-server in the patch failed. |
| +1 :green_heart: | unit | 9m 37s | hbase-mapreduce in the patch passed. |
| | | 183m 21s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux ff28b90d9735 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 / dcb38f47db |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/2/testReport/ |
| Max. process+thread count | 4529 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/2/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r554147880
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreContext.java
##########
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.crypto.Encryption;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.util.ClassSize;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This carries the immutable information and references on some of the meta data about the HStore.
+ * This meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
+ */
+@InterfaceAudience.Private
+public final class StoreContext implements HeapSize {
+ public static final long FIXED_OVERHEAD = ClassSize.estimateBase(HStore.class, false);
+
+ private final int blockSize;
+ private final Encryption.Context encryptionContext;
+ private final CacheConfig cacheConf;
+ private final HRegionFileSystem regionFileSystem;
+ private final CellComparator comparator;
+ private final BloomType bloomFilterType;
+ private final Supplier<Collection<HStoreFile>> compactedFilesSupplier;
+ private final Supplier<InetSocketAddress[]> favoredNodesSupplier;
+ private final ColumnFamilyDescriptor family;
+ private final Path familyStoreDirectoryPath;
+ private final RegionCoprocessorHost coprocessorHost;
+
+ private StoreContext(Builder builder) {
+ this.blockSize = builder.blockSize;
+ this.encryptionContext = builder.encryptionContext;
+ this.cacheConf = builder.cacheConf;
+ this.regionFileSystem = builder.regionFileSystem;
+ this.comparator = builder.comparator;
+ this.bloomFilterType = builder.bloomFilterType;
+ this.compactedFilesSupplier = builder.compactedFilesSupplier;
+ this.favoredNodesSupplier = builder.favoredNodesSupplier;
+ this.family = builder.family;
+ this.familyStoreDirectoryPath = builder.familyStoreDirectoryPath;
+ this.coprocessorHost = builder.coprocessorHost;
+ }
+
+ public int getBlockSize() {
+ return blockSize;
+ }
+
+ public Encryption.Context getEncryptionContext() {
+ return encryptionContext;
+ }
+
+ public CacheConfig getCacheConf() {
+ return cacheConf;
+ }
+
+ public HRegionFileSystem getRegionFileSystem() {
+ return regionFileSystem;
+ }
+
+ public CellComparator getComparator() {
+ return comparator;
+ }
+
+ public BloomType getBloomFilterType() {
+ return bloomFilterType;
+ }
+
+ public Supplier<Collection<HStoreFile>> getCompactedFilesSupplier() {
+ return compactedFilesSupplier;
+ }
+
+ public InetSocketAddress[] getFavoredNodes() {
+ return favoredNodesSupplier.get();
+ }
+
+ public ColumnFamilyDescriptor getFamily() {
+ return family;
+ }
+
+ public Path getFamilyStoreDirectoryPath() {
+ return familyStoreDirectoryPath;
+ }
+
+ public RegionCoprocessorHost getCoprocessorHost() {
+ return coprocessorHost;
+ }
+
+ public static Builder getBuilder() {
+ return new Builder();
+ }
+
+ @Override
+ public long heapSize() {
+ return FIXED_OVERHEAD;
+ }
Review comment:
@apurtell sorry for pinging again, I will merge this evening if I don't hear from you
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r548126048
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private HStoreContext storeContext;
Review comment:
we introduced this context for a reason. `HStore` is a big informative object passed as a input to many file operation classes e.g. `StoreFlusher` to get access for related information in reference, e.g. `columnFamilyDescriptor` and `regionFileSystem`, we're trying to group those `read-only` reference such that we don't have to always use `HStore` object in the lower classes.
Although this refactoring is the base for the upcoming patches in HBASE-24749 that we will introduce new `StoreFileWriterFactory` and `StoreFileCommitter`, we think this could be a good wrapper to group and limit access for internal reference as well.
> (We need the 'H' in HStoreContext?)
we can remove the `H` from naming, will do in the next commit
----------------------------------------------------------------
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] z-york commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
z-york commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r549514178
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreContext.java
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.hadoop.hbase.util.ClassSize;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
+ */
+@InterfaceAudience.Private
+public final class StoreContext implements HeapSize {
+ public static final long FIXED_OVERHEAD = ClassSize.estimateBase(HStore.class, false);
+
+ private final HFileContext defaultFileContext;
+ private final CacheConfig cacheConf;
+ private final HRegionFileSystem regionFileSystem;
+ private final CellComparator comparator;
+ private final BloomType bloomFilterType;
+ private final Supplier<Collection<HStoreFile>> compactedFilesSupplier;
+ private final Supplier<InetSocketAddress[]> favoredNodesSupplier;
+ private final ColumnFamilyDescriptor family;
+ private final Path familyStoreDirectoryPath;
+ private final RegionCoprocessorHost coprocessorHost;
+
+ private StoreContext(Builder builder) {
+ this.defaultFileContext = builder.defaultFileContext;
+ this.cacheConf = builder.cacheConf;
+ this.regionFileSystem = builder.regionFileSystem;
+ this.comparator = builder.comparator;
+ this.bloomFilterType = builder.bloomFilterType;
+ this.compactedFilesSupplier = builder.compactedFilesSupplier;
+ this.favoredNodesSupplier = builder.favoredNodesSupplier;
+ this.family = builder.family;
+ this.familyStoreDirectoryPath = builder.familyStoreDirectoryPath;
+ this.coprocessorHost = builder.coprocessorHost;
+ }
+
+ public HFileContext getDefaultFileContext() {
+ return defaultFileContext;
+ }
+
+ public CacheConfig getCacheConf() {
+ return cacheConf;
+ }
+
+ public HRegionFileSystem getRegionFileSystem() {
+ return regionFileSystem;
+ }
+
+ public CellComparator getComparator() {
+ return comparator;
+ }
+
+ public BloomType getBloomFilterType() {
+ return bloomFilterType;
+ }
+
+ public Supplier<Collection<HStoreFile>> getCompactedFilesSupplier() {
+ return compactedFilesSupplier;
+ }
+
+ public Supplier<InetSocketAddress[]> getFavoredNodesSupplier() {
+ return favoredNodesSupplier;
+ }
Review comment:
I guess the answer to this will depend on whether we expect the contents to change... I see it is passed into the HFileContext... is it evaluated right away or only used when it is needed?
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r548126048
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private HStoreContext storeContext;
Review comment:
we introduced this context for a reason. `HStore` is a big informative object passed as a input to many file operation classes e.g. `StoreFlusher` to get access for related information in reference, e.g. `columnFamilyDescriptor` and `regionFileSystem`, we're trying to group those `read-only` reference such that we don't have to always use `HStore` object in the lower classes.
Although this refactoring is base for the upcoming patches in HBASE-24749 that we will introduce new `StoreFileWriterFactory` and `StoreFileCommitter`, we think this could be a good wrapper to group and limit access for internal reference as well.
> (We need the 'H' in HStoreContext?)
we can remove the `H` from naming, will do in the next commit
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-752292504
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | 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 _ |
| +0 :ok: | mvndep | 0m 30s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 31s | master passed |
| +1 :green_heart: | checkstyle | 1m 46s | master passed |
| +1 :green_heart: | spotbugs | 3m 22s | master passed |
| -0 :warning: | patch | 1m 5s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 30s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 24s | The patch passed checkstyle in hbase-common |
| +1 :green_heart: | checkstyle | 1m 6s | hbase-server: The patch generated 0 new + 42 unchanged - 1 fixed = 42 total (was 43) |
| +1 :green_heart: | checkstyle | 0m 18s | The patch passed checkstyle in hbase-mapreduce |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 17m 9s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 50s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 37s | The patch does not generate ASF License warnings. |
| | | 44m 44s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/8/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 07a20d88ebe1 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 / c96fbf0407 |
| Max. process+thread count | 94 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/8/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r551656408
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -474,33 +501,14 @@ public long getBlockingFileCount() {
}
/* End implementation of StoreConfigInformation */
- /**
- * Returns the configured bytesPerChecksum value.
- * @param conf The configuration
- * @return The bytesPerChecksum that is set in the configuration
- */
- public static int getBytesPerChecksum(Configuration conf) {
- return conf.getInt(HConstants.BYTES_PER_CHECKSUM,
- HFile.DEFAULT_BYTES_PER_CHECKSUM);
- }
-
- /**
- * Returns the configured checksum algorithm.
- * @param conf The configuration
- * @return The checksum algorithm that is set in the configuration
- */
- public static ChecksumType getChecksumType(Configuration conf) {
- String checksumName = conf.get(HConstants.CHECKSUM_TYPE_NAME);
- if (checksumName == null) {
- return ChecksumType.getDefaultChecksumType();
- } else {
- return ChecksumType.nameToType(checksumName);
- }
- }
@Override
public ColumnFamilyDescriptor getColumnFamilyDescriptor() {
- return this.family;
+ return this.storeContext.getFamily();
+ }
+
+ public Encryption.Context getCryptoContext() {
Review comment:
I have changed that in the latest update, please see if you have more comments, and I marked it as resolved first
----------------------------------------------------------------
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] z-york commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
z-york commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-754322209
LGTM. +1
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r546914142
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -474,33 +501,14 @@ public long getBlockingFileCount() {
}
/* End implementation of StoreConfigInformation */
- /**
- * Returns the configured bytesPerChecksum value.
- * @param conf The configuration
- * @return The bytesPerChecksum that is set in the configuration
- */
- public static int getBytesPerChecksum(Configuration conf) {
- return conf.getInt(HConstants.BYTES_PER_CHECKSUM,
- HFile.DEFAULT_BYTES_PER_CHECKSUM);
- }
-
- /**
- * Returns the configured checksum algorithm.
- * @param conf The configuration
- * @return The checksum algorithm that is set in the configuration
- */
- public static ChecksumType getChecksumType(Configuration conf) {
- String checksumName = conf.get(HConstants.CHECKSUM_TYPE_NAME);
- if (checksumName == null) {
- return ChecksumType.getDefaultChecksumType();
- } else {
- return ChecksumType.nameToType(checksumName);
- }
- }
@Override
public ColumnFamilyDescriptor getColumnFamilyDescriptor() {
- return this.family;
+ return this.storeContext.getFamily();
+ }
+
+ public Encryption.Context getCryptoContext() {
Review comment:
Consider naming this `getEncryptionContext` (as elsewhere)
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -2559,7 +2572,7 @@ public boolean needsCompaction() {
* @return cache configuration for this Store.
*/
public CacheConfig getCacheConfig() {
- return this.cacheConf;
+ return storeContext.getCacheConf();
}
public static final long FIXED_OVERHEAD = ClassSize.estimateBase(HStore.class, false);
Review comment:
Does DEEP_OVERHEAD and heapSize() need to change? Does TestHeapSize still pass?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStoreContext.java
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
+ */
+@InterfaceAudience.Private
+public class HStoreContext {
Review comment:
Should this implement HeapSize? This information used to be included in Store/HStore heap utilization estimation via HStore#heapSize() and so we should still track it?
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749848750
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | 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 _ |
| +0 :ok: | mvndep | 0m 28s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 31s | master passed |
| +1 :green_heart: | compile | 1m 25s | master passed |
| +1 :green_heart: | shadedjars | 6m 39s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 53s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 30s | the patch passed |
| +1 :green_heart: | compile | 1m 20s | the patch passed |
| +1 :green_heart: | javac | 1m 20s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 29s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 138m 39s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 10m 9s | hbase-mapreduce in the patch passed. |
| | | 177m 3s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d946bbe4f0b4 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 / dcb38f47db |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/2/testReport/ |
| Max. process+thread count | 4603 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/2/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749168577
:broken_heart: **-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. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 22s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 44s | master passed |
| +1 :green_heart: | checkstyle | 1m 24s | master passed |
| +1 :green_heart: | spotbugs | 2m 48s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 31s | the patch passed |
| -0 :warning: | checkstyle | 1m 4s | hbase-server: The patch generated 7 new + 39 unchanged - 0 fixed = 46 total (was 39) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 3s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 6s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | asflicense | 0m 26s | The patch generated 1 ASF License warnings. |
| | | 45m 6s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 64c3a8c368a5 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 / dcb38f47db |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| asflicense | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/artifact/yetus-general-check/output/patch-asflicense-problems.txt |
| Max. process+thread count | 94 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/1/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-750473295
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 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. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 36s | master passed |
| +1 :green_heart: | checkstyle | 1m 45s | master passed |
| +1 :green_heart: | spotbugs | 3m 18s | master passed |
| -0 :warning: | patch | 2m 42s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 23s | the patch passed |
| -0 :warning: | checkstyle | 1m 15s | hbase-server: The patch generated 1 new + 42 unchanged - 1 fixed = 43 total (was 43) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 21m 54s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 47s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 25s | The patch does not generate ASF License warnings. |
| | | 52m 44s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 56c49b02a485 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dcb38f47db |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/5/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552203329
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreUtils.java
##########
@@ -136,4 +140,26 @@ public static OptionalLong getMaxSequenceIdInList(Collection<HStoreFile> sfs) {
return largestFile.isPresent() ? StoreUtils.getFileSplitPoint(largestFile.get(), comparator)
: Optional.empty();
}
+
+ /**
+ * Returns the configured checksum algorithm.
+ * @param conf The configuration
+ * @return The checksum algorithm that is set in the configuration
+ */
+ public static ChecksumType getChecksumType(Configuration conf) {
+ String checksumName = conf.get(HConstants.CHECKSUM_TYPE_NAME);
Review comment:
@saintstack we have updated it in the last commit, do you mind to have a second look?
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552887851
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -308,7 +294,7 @@ protected HStore(final HRegion region, final ColumnFamilyDescriptor family,
this.compactionCheckMultiplier = DEFAULT_COMPACTCHECKER_INTERVAL_MULTIPLIER;
}
- this.storeEngine = createStoreEngine(this, this.conf, this.comparator);
+ this.storeEngine = createStoreEngine(this, this.conf, getComparator());
Review comment:
changed to use `region.getCellComparator()`.
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755210852
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 36s | 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 _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 20s | master passed |
| +1 :green_heart: | compile | 2m 9s | master passed |
| +1 :green_heart: | shadedjars | 9m 2s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 16s | master passed |
| -0 :warning: | patch | 10m 42s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 18s | the patch passed |
| +1 :green_heart: | compile | 2m 6s | the patch passed |
| +1 :green_heart: | javac | 2m 6s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 2s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 12s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 154m 57s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 11m 45s | hbase-mapreduce in the patch passed. |
| | | 205m 44s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/10/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux f7476d262e72 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 / a5eb8f1f70 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/10/testReport/ |
| Max. process+thread count | 4573 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/10/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-756624495
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 23s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 49s | master passed |
| +1 :green_heart: | compile | 1m 40s | master passed |
| +1 :green_heart: | shadedjars | 7m 25s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 4s | master passed |
| -0 :warning: | patch | 8m 48s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 37s | the patch passed |
| +1 :green_heart: | compile | 1m 40s | the patch passed |
| +1 :green_heart: | javac | 1m 40s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 24s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 2s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 197m 23s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 14m 21s | hbase-mapreduce in the patch passed. |
| | | 245m 37s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 3f604015ea00 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2444d26890 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/testReport/ |
| Max. process+thread count | 3384 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/13/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] saintstack commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552255807
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileContext.java
##########
@@ -138,6 +138,10 @@ public boolean isCompressedOrEncrypted() {
return compressAlgo;
}
+ public void setCompression(Compression.Algorithm compressAlgo) {
+ this.compressAlgo = compressAlgo;
+ }
+
Review comment:
Whats going on here?
HFileContext is supposed to be ' Read-only HFile Context Information' but here we are adding a setter. I see there are one or two setters but we also have HFileContextBuilder. Shouldn't we be going via the Builder making HFileContexts? And why is this method not added on the Builder? (Can it be added non-public to encourage users to go via the Builder)?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -254,48 +244,47 @@
protected HStore(final HRegion region, final ColumnFamilyDescriptor family,
final Configuration confParam, boolean warmup) throws IOException {
- this.fs = region.getRegionFileSystem();
-
- // Assemble the store's home directory and Ensure it exists.
- fs.createStoreDir(family.getNameAsString());
- this.region = region;
- this.family = family;
// 'conf' renamed to 'confParam' b/c we use this.conf in the constructor
// CompoundConfiguration will look for keys in reverse order of addition, so we'd
// add global config first, then table and cf overrides, then cf metadata.
this.conf = new CompoundConfiguration()
- .add(confParam)
- .addBytesMap(region.getTableDescriptor().getValues())
- .addStringMap(family.getConfiguration())
- .addBytesMap(family.getValues());
- this.blocksize = family.getBlocksize();
+ .add(confParam)
+ .addBytesMap(region.getTableDescriptor().getValues())
+ .addStringMap(family.getConfiguration())
+ .addBytesMap(family.getValues());
+
+ this.region = region;
+ this.storeContext = initializeStoreContext(family);
+
+ // Assemble the store's home directory and Ensure it exists.
+ getRegionFileSystem().createStoreDir(getColumnFamilyName());
+
+ this.blocksize = getColumnFamilyDescriptor().getBlocksize();
// set block storage policy for store directory
- String policyName = family.getStoragePolicy();
+ String policyName = getColumnFamilyDescriptor().getStoragePolicy();
Review comment:
nit: why bother with this change at all? 'family' is passed on the constructor. Its final. Why bother going via the accessor in the constructor?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreContext.java
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.hadoop.hbase.util.ClassSize;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
Review comment:
I would like to know here in class comment if this is read-only/immutable. I think it should be.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreContext.java
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.hadoop.hbase.util.ClassSize;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
Review comment:
Also, just a comment... the HFileContext has this for a comment...
* Read-only HFile Context Information. Meta data that is used by HFileWriter/Readers and by
* HFileBlocks. Create one using the {@link HFileContextBuilder} (See HFileInfo and the HFile
* Trailer class).
so it is for readers and writers.... So, StoreContext is probably fine but if you are wondering... here is incidences of Info.java vs Context.java.... If it helps.
```
kalashnikov:hbase.apache.git stack$ find src/main/java -name *Info.java
find: src/main/java: No such file or directory
kalashnikov:hbase.apache.git stack$ find hbase-*/src/main/java -name *Info.java
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupTableInfo.java
hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java
hbase-client/src/main/java/org/apache/hadoop/hbase/security/SecurityInfo.java
hbase-client/src/main/java/org/apache/hadoop/hbase/client/MutableRegionInfo.java
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
hbase-common/src/main/java/org/apache/hadoop/hbase/util/VersionInfo.java
hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/mode/DrillDownInfo.java
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/field/FieldInfo.java
hbase-metrics-api/src/main/java/org/apache/hadoop/hbase/metrics/MetricRegistryInfo.java
hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ReplicationQueueInfo.java
hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotInfo.java
hbase-server/src/main/java/org/apache/hadoop/hbase/util/HbckTableInfo.java
hbase-server/src/main/java/org/apache/hadoop/hbase/util/HbckRegionInfo.java
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileInfo.java
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/BlockWithScanInfo.java
hbase-server/src/main/java/org/apache/hadoop/hbase/namespace/NamespaceTableAndRegionInfo.java
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanInfo.java
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFileInfo.java
hbase-server/src/main/java/org/apache/hadoop/hbase/master/webapp/RegionReplicaInfo.java
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/CallQueueInfo.java
hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/generated/THRegionInfo.java
hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/generated/TRegionInfo.java
```
```
kalashnikov:hbase.apache.git stack$ find hbase-*/src/main/java -name *Context.java
hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/Context.java
hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/HFileBlockDecodingContext.java
hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/HFileBlockEncodingContext.java
hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/HFileBlockDefaultDecodingContext.java
hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/HFileBlockDefaultEncodingContext.java
hbase-common/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileContext.java
hbase-common/src/main/java/org/apache/hadoop/hbase/io/TagCompressionContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/util/RowPrefixFixedLengthBloomContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/util/RowBloomContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/util/RowColBloomContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/util/BloomContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/ReaderContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/ObserverContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/CompressionContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScannerContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactionContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/NoLimitScannerContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFlushContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcCallContext.java
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcSchedulerContext.java
```
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -347,6 +331,48 @@ protected HStore(final HRegion region, final ColumnFamilyDescriptor family,
cacheOnWriteLogged = false;
}
+ private StoreContext initializeStoreContext(ColumnFamilyDescriptor family) throws IOException {
+ return new StoreContext.Builder()
+ .withBloomType(family.getBloomFilterType())
+ .withCacheConfig(createCacheConf(family))
+ .withCellComparator(region.getCellComparator())
+ .withColumnFamilyDescriptor(family)
+ .withCompactedFilesSupplier(this::getCompactedFiles)
+ .withRegionFileSystem(region.getRegionFileSystem())
+ .withDefaultHFileContext(getDefaultHFileContext(family))
+ .withFavoredNodesSupplier(this::getFavoredNodes)
+ .withFamilyStoreDirectoryPath(region.getRegionFileSystem()
+ .getStoreDir(family.getNameAsString()))
+ .withRegionCoprocessorHost(region.getCoprocessorHost())
+ .build();
+ }
+
+ private InetSocketAddress[] getFavoredNodes() {
+ InetSocketAddress[] favoredNodes = null;
+ if (region.getRegionServerServices() != null) {
+ favoredNodes = region.getRegionServerServices().getFavoredNodesForRegion(
+ region.getRegionInfo().getEncodedName());
+ }
+ return favoredNodes;
+ }
+
+ private HFileContext getDefaultHFileContext(ColumnFamilyDescriptor family) throws IOException {
Review comment:
Could be static?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1206,53 +1218,34 @@ public StoreFileWriter createWriterInTmp(long maxKeyCount, Compression.Algorithm
}
}
}
- InetSocketAddress[] favoredNodes = null;
- if (region.getRegionServerServices() != null) {
- favoredNodes = region.getRegionServerServices().getFavoredNodesForRegion(
- region.getRegionInfo().getEncodedName());
- }
- HFileContext hFileContext = createFileContext(compression, includeMVCCReadpoint, includesTag,
- cryptoContext);
- Path familyTempDir = new Path(fs.getTempDir(), family.getNameAsString());
- StoreFileWriter.Builder builder = new StoreFileWriter.Builder(conf, writerCacheConf,
- this.getFileSystem())
- .withOutputDir(familyTempDir)
- .withBloomType(family.getBloomFilterType())
- .withMaxKeyCount(maxKeyCount)
- .withFavoredNodes(favoredNodes)
- .withFileContext(hFileContext)
- .withShouldDropCacheBehind(shouldDropBehind)
- .withCompactedFilesSupplier(this::getCompactedFiles)
- .withFileStoragePolicy(fileStoragePolicy);
+ HFileContext hFileContext = createFileContext(compression, includeMVCCReadpoint, includesTag);
+ Path familyTempDir = new Path(getRegionFileSystem().getTempDir(), getColumnFamilyName());
+ StoreFileWriter.Builder builder =
+ new StoreFileWriter.Builder(conf, writerCacheConf, getFileSystem())
+ .withOutputDir(familyTempDir)
+ .withBloomType(storeContext.getBloomFilterType())
+ .withMaxKeyCount(maxKeyCount)
+ .withFavoredNodes(storeContext.getFavoredNodes())
+ .withFileContext(hFileContext)
+ .withShouldDropCacheBehind(shouldDropBehind)
+ .withCompactedFilesSupplier(storeContext.getCompactedFilesSupplier())
+ .withFileStoragePolicy(fileStoragePolicy);
return builder.build();
}
private HFileContext createFileContext(Compression.Algorithm compression,
- boolean includeMVCCReadpoint, boolean includesTag, Encryption.Context cryptoContext) {
+ boolean includeMVCCReadpoint, boolean includesTag) {
if (compression == null) {
compression = HFile.DEFAULT_COMPRESSION_ALGORITHM;
}
- HFileContext hFileContext = new HFileContextBuilder()
- .withIncludesMvcc(includeMVCCReadpoint)
- .withIncludesTags(includesTag)
- .withCompression(compression)
- .withCompressTags(family.isCompressTags())
- .withChecksumType(checksumType)
- .withBytesPerCheckSum(bytesPerChecksum)
- .withBlockSize(blocksize)
- .withHBaseCheckSum(true)
- .withDataBlockEncoding(family.getDataBlockEncoding())
- .withEncryptionContext(cryptoContext)
- .withCreateTime(EnvironmentEdgeManager.currentTime())
- .withColumnFamily(family.getName())
- .withTableName(region.getTableDescriptor()
- .getTableName().getName())
- .withCellComparator(this.comparator)
- .build();
- return hFileContext;
+ HFileContext fileContext = storeContext.getDefaultFileContext();
+ fileContext.setIncludesMvcc(includeMVCCReadpoint);
+ fileContext.setIncludesTags(includesTag);
+ fileContext.setCompression(compression);
+ fileContext.setFileCreateTime(EnvironmentEdgeManager.currentTime());
Review comment:
Why would we do this here and not as methods on the builder?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -474,33 +502,14 @@ public long getBlockingFileCount() {
}
/* End implementation of StoreConfigInformation */
- /**
- * Returns the configured bytesPerChecksum value.
- * @param conf The configuration
- * @return The bytesPerChecksum that is set in the configuration
- */
- public static int getBytesPerChecksum(Configuration conf) {
- return conf.getInt(HConstants.BYTES_PER_CHECKSUM,
- HFile.DEFAULT_BYTES_PER_CHECKSUM);
- }
-
- /**
- * Returns the configured checksum algorithm.
- * @param conf The configuration
- * @return The checksum algorithm that is set in the configuration
- */
- public static ChecksumType getChecksumType(Configuration conf) {
- String checksumName = conf.get(HConstants.CHECKSUM_TYPE_NAME);
- if (checksumName == null) {
- return ChecksumType.getDefaultChecksumType();
- } else {
- return ChecksumType.nameToType(checksumName);
- }
- }
@Override
public ColumnFamilyDescriptor getColumnFamilyDescriptor() {
- return this.family;
+ return this.storeContext.getFamily();
+ }
+
+ public Encryption.Context getEncryptionContext() {
Review comment:
Has to be public? Has to be on HStore? Can it be on StoreContext? Is there a getStoreContext method on HStore?
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552887639
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -268,34 +251,37 @@ protected HStore(final HRegion region, final ColumnFamilyDescriptor family,
.addBytesMap(region.getTableDescriptor().getValues())
.addStringMap(family.getConfiguration())
.addBytesMap(family.getValues());
- this.blocksize = family.getBlocksize();
+
+ this.region = region;
+ this.storeContext = initializeStoreContext(family);
+
+ // Assemble the store's home directory and Ensure it exists.
+ getRegionFileSystem().createStoreDir(family.getNameAsString());
// set block storage policy for store directory
String policyName = family.getStoragePolicy();
if (null == policyName) {
policyName = this.conf.get(BLOCK_STORAGE_POLICY_KEY, DEFAULT_BLOCK_STORAGE_POLICY);
}
- this.fs.setStoragePolicy(family.getNameAsString(), policyName.trim());
+ getRegionFileSystem().setStoragePolicy(getColumnFamilyName(), policyName.trim());
Review comment:
changed back to use `family` and `region.getRegionFileSystem()` directly.
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749802781
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | 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 _ |
| +0 :ok: | mvndep | 0m 27s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 30s | master passed |
| +1 :green_heart: | checkstyle | 1m 21s | master passed |
| +1 :green_heart: | spotbugs | 2m 42s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 31s | the patch passed |
| -0 :warning: | checkstyle | 1m 5s | hbase-server: The patch generated 9 new + 42 unchanged - 1 fixed = 51 total (was 43) |
| +1 :green_heart: | whitespace | 0m 1s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 41s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 26s | The patch does not generate ASF License warnings. |
| | | 45m 58s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux a6e90bd5c539 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 / dcb38f47db |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 94 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/2/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r548317844
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private HStoreContext storeContext;
Review comment:
IMO those informative accessor/reference and final/read-only primitives should ideally be in the context, although we're focusing on writer (`StoreFileWriter`) related reference and may have missed few of them (e.g. `scanInfo`) in this commit.
let's try to clarify your suggestion
1. if you see the `StoreContext` is general to be applied on most cases, are those missing fields (e.g. `scanInfo` and final primitives) what you're trying to point out ? if so, we can revisit and filter/add more into the `StoreContext`
2. The scope of this `Context` is more related to Writer(`StoreFileWriter`)/Committer(will be added), should we rename it to `StoreWriterContext`/`StoreWriteContext` that used by those operators?
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552986081
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreContext.java
##########
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.crypto.Encryption;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.util.ClassSize;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This carries the immutable information and references on some of the meta data about the HStore.
+ * This meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
+ */
+@InterfaceAudience.Private
+public final class StoreContext implements HeapSize {
+ public static final long FIXED_OVERHEAD = ClassSize.estimateBase(HStore.class, false);
+
+ private final int blockSize;
+ private final Encryption.Context encryptionContext;
+ private final CacheConfig cacheConf;
+ private final HRegionFileSystem regionFileSystem;
+ private final CellComparator comparator;
+ private final BloomType bloomFilterType;
+ private final Supplier<Collection<HStoreFile>> compactedFilesSupplier;
+ private final Supplier<InetSocketAddress[]> favoredNodesSupplier;
+ private final ColumnFamilyDescriptor family;
+ private final Path familyStoreDirectoryPath;
+ private final RegionCoprocessorHost coprocessorHost;
+
+ private StoreContext(Builder builder) {
+ this.blockSize = builder.blockSize;
+ this.encryptionContext = builder.encryptionContext;
+ this.cacheConf = builder.cacheConf;
+ this.regionFileSystem = builder.regionFileSystem;
+ this.comparator = builder.comparator;
+ this.bloomFilterType = builder.bloomFilterType;
+ this.compactedFilesSupplier = builder.compactedFilesSupplier;
+ this.favoredNodesSupplier = builder.favoredNodesSupplier;
+ this.family = builder.family;
+ this.familyStoreDirectoryPath = builder.familyStoreDirectoryPath;
+ this.coprocessorHost = builder.coprocessorHost;
+ }
+
+ public int getBlockSize() {
+ return blockSize;
+ }
+
+ public Encryption.Context getEncryptionContext() {
+ return encryptionContext;
+ }
+
+ public CacheConfig getCacheConf() {
+ return cacheConf;
+ }
+
+ public HRegionFileSystem getRegionFileSystem() {
+ return regionFileSystem;
+ }
+
+ public CellComparator getComparator() {
+ return comparator;
+ }
+
+ public BloomType getBloomFilterType() {
+ return bloomFilterType;
+ }
+
+ public Supplier<Collection<HStoreFile>> getCompactedFilesSupplier() {
+ return compactedFilesSupplier;
+ }
+
+ public InetSocketAddress[] getFavoredNodes() {
+ return favoredNodesSupplier.get();
+ }
+
+ public ColumnFamilyDescriptor getFamily() {
+ return family;
+ }
+
+ public Path getFamilyStoreDirectoryPath() {
+ return familyStoreDirectoryPath;
+ }
+
+ public RegionCoprocessorHost getCoprocessorHost() {
+ return coprocessorHost;
+ }
+
+ public static Builder getBuilder() {
+ return new Builder();
+ }
+
+ @Override
+ public long heapSize() {
+ return FIXED_OVERHEAD;
+ }
Review comment:
@apurtell ping again on the `heapSize()` and implements `HeapSize` for StoreContext class, could you have another look? I should have fixed it, but will wait for your approval/resolve in 1 day or 2 day before merging it.
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755959907
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 23s | 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 _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 7s | master passed |
| +1 :green_heart: | compile | 1m 25s | master passed |
| +1 :green_heart: | shadedjars | 7m 10s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 56s | master passed |
| -0 :warning: | patch | 8m 24s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 49s | the patch passed |
| +1 :green_heart: | compile | 1m 22s | the patch passed |
| +1 :green_heart: | javac | 1m 22s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 12s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 53s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 208m 32s | hbase-server in the patch failed. |
| +1 :green_heart: | unit | 15m 35s | hbase-mapreduce in the patch passed. |
| | | 254m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/12/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4ea5fac95444 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a414361ed9 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/12/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/12/testReport/ |
| Max. process+thread count | 4072 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/12/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu commented on a change in pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r548162792
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreUtils.java
##########
@@ -136,4 +140,26 @@ public static OptionalLong getMaxSequenceIdInList(Collection<HStoreFile> sfs) {
return largestFile.isPresent() ? StoreUtils.getFileSplitPoint(largestFile.get(), comparator)
: Optional.empty();
}
+
+ /**
+ * Returns the configured checksum algorithm.
+ * @param conf The configuration
+ * @return The checksum algorithm that is set in the configuration
+ */
+ public static ChecksumType getChecksumType(Configuration conf) {
+ String checksumName = conf.get(HConstants.CHECKSUM_TYPE_NAME);
Review comment:
will change in next commit. we put it as it's a static helper method, IMO we're writing it in the right Utils class?
----------------------------------------------------------------
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] saintstack commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552381002
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1206,53 +1218,34 @@ public StoreFileWriter createWriterInTmp(long maxKeyCount, Compression.Algorithm
}
}
}
- InetSocketAddress[] favoredNodes = null;
- if (region.getRegionServerServices() != null) {
- favoredNodes = region.getRegionServerServices().getFavoredNodesForRegion(
- region.getRegionInfo().getEncodedName());
- }
- HFileContext hFileContext = createFileContext(compression, includeMVCCReadpoint, includesTag,
- cryptoContext);
- Path familyTempDir = new Path(fs.getTempDir(), family.getNameAsString());
- StoreFileWriter.Builder builder = new StoreFileWriter.Builder(conf, writerCacheConf,
- this.getFileSystem())
- .withOutputDir(familyTempDir)
- .withBloomType(family.getBloomFilterType())
- .withMaxKeyCount(maxKeyCount)
- .withFavoredNodes(favoredNodes)
- .withFileContext(hFileContext)
- .withShouldDropCacheBehind(shouldDropBehind)
- .withCompactedFilesSupplier(this::getCompactedFiles)
- .withFileStoragePolicy(fileStoragePolicy);
+ HFileContext hFileContext = createFileContext(compression, includeMVCCReadpoint, includesTag);
+ Path familyTempDir = new Path(getRegionFileSystem().getTempDir(), getColumnFamilyName());
+ StoreFileWriter.Builder builder =
+ new StoreFileWriter.Builder(conf, writerCacheConf, getFileSystem())
+ .withOutputDir(familyTempDir)
+ .withBloomType(storeContext.getBloomFilterType())
+ .withMaxKeyCount(maxKeyCount)
+ .withFavoredNodes(storeContext.getFavoredNodes())
+ .withFileContext(hFileContext)
+ .withShouldDropCacheBehind(shouldDropBehind)
+ .withCompactedFilesSupplier(storeContext.getCompactedFilesSupplier())
+ .withFileStoragePolicy(fileStoragePolicy);
return builder.build();
}
private HFileContext createFileContext(Compression.Algorithm compression,
- boolean includeMVCCReadpoint, boolean includesTag, Encryption.Context cryptoContext) {
+ boolean includeMVCCReadpoint, boolean includesTag) {
if (compression == null) {
compression = HFile.DEFAULT_COMPRESSION_ALGORITHM;
}
- HFileContext hFileContext = new HFileContextBuilder()
- .withIncludesMvcc(includeMVCCReadpoint)
- .withIncludesTags(includesTag)
- .withCompression(compression)
- .withCompressTags(family.isCompressTags())
- .withChecksumType(checksumType)
- .withBytesPerCheckSum(bytesPerChecksum)
- .withBlockSize(blocksize)
- .withHBaseCheckSum(true)
- .withDataBlockEncoding(family.getDataBlockEncoding())
- .withEncryptionContext(cryptoContext)
- .withCreateTime(EnvironmentEdgeManager.currentTime())
- .withColumnFamily(family.getName())
- .withTableName(region.getTableDescriptor()
- .getTableName().getName())
- .withCellComparator(this.comparator)
- .build();
- return hFileContext;
+ HFileContext fileContext = storeContext.getDefaultFileContext();
+ fileContext.setIncludesMvcc(includeMVCCReadpoint);
+ fileContext.setIncludesTags(includesTag);
+ fileContext.setCompression(compression);
+ fileContext.setFileCreateTime(EnvironmentEdgeManager.currentTime());
Review comment:
That'd be better. If a builder, lets use it everywhere (can add a constructor on builder that allows a bit of shortcutting auto-filling most fields....)
----------------------------------------------------------------
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] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r551656590
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -2559,7 +2572,7 @@ public boolean needsCompaction() {
* @return cache configuration for this Store.
*/
public CacheConfig getCacheConfig() {
- return this.cacheConf;
+ return storeContext.getCacheConf();
}
public static final long FIXED_OVERHEAD = ClassSize.estimateBase(HStore.class, false);
Review comment:
this has been change as well, can you have another look ?
----------------------------------------------------------------
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] saintstack commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552380797
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreContext.java
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.HeapSize;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.hadoop.hbase.util.ClassSize;
+import org.apache.yetus.audience.InterfaceAudience;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore consumers without the
+ * need of passing around the complete store.
Review comment:
Sounds good.
----------------------------------------------------------------
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] taklwu commented on pull request #2800: HBASE-25249 Adding HStoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-749784957
@apurtell requested changes updated.
I shouldn't rebase and squash for the updated changes, will be providing a new diff on top of the first/original change next time
----------------------------------------------------------------
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 #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-750694698
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | 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 _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 31s | master passed |
| +1 :green_heart: | compile | 1m 23s | master passed |
| +1 :green_heart: | shadedjars | 6m 37s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 56s | master passed |
| -0 :warning: | patch | 7m 55s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 18s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 27s | the patch passed |
| +1 :green_heart: | compile | 1m 20s | the patch passed |
| +1 :green_heart: | javac | 1m 20s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 35s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 56s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 138m 10s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 10m 11s | hbase-mapreduce in the patch passed. |
| | | 176m 32s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 75dd5f221591 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 / dcb38f47db |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/6/testReport/ |
| Max. process+thread count | 4802 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/6/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755198847
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | 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 _ |
| +0 :ok: | mvndep | 0m 27s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 54s | master passed |
| +1 :green_heart: | compile | 1m 25s | master passed |
| +1 :green_heart: | shadedjars | 6m 48s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 55s | master passed |
| -0 :warning: | patch | 8m 7s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 18s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 31s | the patch passed |
| +1 :green_heart: | compile | 1m 25s | the patch passed |
| +1 :green_heart: | javac | 1m 25s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 34s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 54s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 143m 27s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 11m 5s | hbase-mapreduce in the patch passed. |
| | | 183m 45s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/10/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux f2d5b8dfff1f 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 / a5eb8f1f70 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/10/testReport/ |
| Max. process+thread count | 4257 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/10/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-750543803
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 13s | 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 _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 25s | master passed |
| +1 :green_heart: | checkstyle | 1m 40s | master passed |
| +1 :green_heart: | spotbugs | 3m 13s | master passed |
| -0 :warning: | patch | 1m 4s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 5s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 11s | hbase-server: The patch generated 0 new + 42 unchanged - 1 fixed = 42 total (was 43) |
| +1 :green_heart: | checkstyle | 0m 19s | The patch passed checkstyle in hbase-mapreduce |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 41s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 9s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 21s | The patch does not generate ASF License warnings. |
| | | 48m 54s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux b094a34e7a9c 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dcb38f47db |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/6/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755883881
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 38s | 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 _ |
| +0 :ok: | mvndep | 0m 29s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 44s | master passed |
| +1 :green_heart: | checkstyle | 1m 41s | master passed |
| +1 :green_heart: | spotbugs | 3m 30s | master passed |
| -0 :warning: | patch | 1m 10s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 29s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 31s | hbase-server: The patch generated 0 new + 42 unchanged - 1 fixed = 42 total (was 43) |
| +1 :green_heart: | checkstyle | 0m 20s | The patch passed checkstyle in hbase-mapreduce |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 24m 0s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 4m 3s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 27s | The patch does not generate ASF License warnings. |
| | | 55m 48s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/12/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux caa9fb3d873d 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 / a414361ed9 |
| Max. process+thread count | 95 (vs. ulimit of 30000) |
| modules | C: hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/12/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#issuecomment-755084616
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | 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 _ |
| +0 :ok: | mvndep | 0m 28s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 18s | master passed |
| +1 :green_heart: | compile | 2m 6s | master passed |
| +1 :green_heart: | shadedjars | 6m 50s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 27s | master passed |
| -0 :warning: | patch | 8m 48s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 17s | the patch passed |
| +1 :green_heart: | compile | 2m 8s | the patch passed |
| +1 :green_heart: | javac | 2m 8s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 52s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 59s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 149m 28s | hbase-server in the patch passed. |
| +1 :green_heart: | unit | 10m 24s | hbase-mapreduce in the patch passed. |
| | | 194m 57s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/9/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2800 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 520455db2aef 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 / 90ff550000 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/9/testReport/ |
| Max. process+thread count | 4577 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server hbase-mapreduce U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2800/9/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] taklwu commented on a change in pull request #2800: HBASE-25249 Adding StoreContext
Posted by GitBox <gi...@apache.org>.
taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r552205785
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -246,6 +234,8 @@
private AtomicLong compactedCellsSize = new AtomicLong();
private AtomicLong majorCompactedCellsSize = new AtomicLong();
+ private HStoreContext storeContext;
Review comment:
@saintstack , do you have other comments about the scope of this StoreContext ? do we need to rename it ? or do you think we could move forward and introduce this context object with the proposed set of informative accessor/reference ?
----------------------------------------------------------------
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