[jira] [Commented] (HBASE-23822) Fix typo in procedures.jsp
[
https://issues.apache.org/jira/browse/HBASE-23822?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033937#comment-17033937
]
Hudson commented on HBASE-23822:
Results for branch branch-2
[build #2464 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//JDK8_Nightly_Build_Report_(Hadoop2)/]
(/) {color:green}+1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> Fix typo in procedures.jsp
> --
>
> Key: HBASE-23822
> URL: https://issues.apache.org/jira/browse/HBASE-23822
> Project: HBase
> Issue Type: Improvement
> Components: website
>Affects Versions: master
>Reporter: Zhuoyue Huang
>Assignee: Zhuoyue Huang
>Priority: Trivial
> Fix For: 3.0.0, 2.3.0, 2.2.4
>
> Attachments: procedures.jsp
>
>
> I think there's a typo.In
> hbase-server/src/main/resources/hbase-webapps/master/procedures.jsp. The
> Sentences "We do not list Procedures that have completed SUCCESSfully; their
> number makes it hard to spot the problematics." are misspelled and need to be
> corrected as "We do not list procedures that have completed successfully;
> their number makes it hard to spot the problematics."
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (HBASE-23814) Add null checks and logging to misc set of tests
[
https://issues.apache.org/jira/browse/HBASE-23814?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033938#comment-17033938
]
Hudson commented on HBASE-23814:
Results for branch branch-2
[build #2464 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//JDK8_Nightly_Build_Report_(Hadoop2)/]
(/) {color:green}+1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> Add null checks and logging to misc set of tests
>
>
> Key: HBASE-23814
> URL: https://issues.apache.org/jira/browse/HBASE-23814
> Project: HBase
> Issue Type: Bug
>Reporter: Michael Stack
>Assignee: Michael Stack
>Priority: Trivial
> Fix For: 3.0.0, 2.3.0
>
>
> I've been studying unit tests of late. A few are failing but then the output
> is missing a detail or shutdown complains of NPE because startup didn't
> succeed.
> Here are super minor items I've been carrying around that I'd like to land.
> They do not change the function of tests (there is an attempt at a fix of
> TestLogsCleaner).
> * TestFullLogReconstruction log the server we've chosen to expire and then
> note where we starting counting rows
> * TestAsyncTableScanException use a define for row counts; count 100 instead
> of 1000 and see if helps
> * TestRawAsyncTableLimitedScanWithFilter check connection was made before
> closing it in tearDown
> * TestLogsCleaner use single mod time. Make it for sure less than now in case
> test runs all in the same millisecond (would cause test fail)
> * TestReplicationBase test table is non-null before closing in tearDown
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (HBASE-23815) [Flakey Test] AbstractTestAsyncTableRegionReplicasRead family of tests fails with no breadcrumbs on why
[
https://issues.apache.org/jira/browse/HBASE-23815?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033939#comment-17033939
]
Hudson commented on HBASE-23815:
Results for branch branch-2
[build #2464 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//JDK8_Nightly_Build_Report_(Hadoop2)/]
(/) {color:green}+1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> [Flakey Test] AbstractTestAsyncTableRegionReplicasRead family of tests fails
> with no breadcrumbs on why
> ---
>
> Key: HBASE-23815
> URL: https://issues.apache.org/jira/browse/HBASE-23815
> Project: HBase
> Issue Type: Bug
> Components: flakies
>Reporter: Michael Stack
>Assignee: Michael Stack
>Priority: Major
> Fix For: 3.0.0, 2.3.0
>
>
> Running locally w/ upped forkcount and maven threading > 1, the set of tests
> that are derived from AbstractTestAsyncTableRegionReplicasRead fail near each
> time often the only ones failed (Get and Scan tests). They don't give clues
> why died. The xml has no info in it and no -output.txt is dropped. The .txt
> just says failed startup.
> I've tried giving the forked jvms more RAM, messed w/ file counts
> Removing all of the custom setup seems to be a breakthrough. Let me put up a
> patch.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (HBASE-23816) [Flakey Test] TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:91) Wrong FS!
[
https://issues.apache.org/jira/browse/HBASE-23816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033940#comment-17033940
]
Hudson commented on HBASE-23816:
Results for branch branch-2
[build #2464 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//JDK8_Nightly_Build_Report_(Hadoop2)/]
(/) {color:green}+1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> [Flakey Test]
> TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:91)
> Wrong FS!
>
>
> Key: HBASE-23816
> URL: https://issues.apache.org/jira/browse/HBASE-23816
> Project: HBase
> Issue Type: Sub-task
>Reporter: Michael Stack
>Assignee: Michael Stack
>Priority: Major
> Fix For: 3.0.0, 2.3.0
>
>
> Can't repro locally w/o running with lots of parallelism. Follow-on from
> parent issue.
> Here is error:
> {code}
> [ERROR] Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed:
> 115.741 s <<< FAILURE! - in
> org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster
> [ERROR]
> org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState
> Time elapsed: 74.459 s <<< ERROR!
> java.lang.IllegalArgumentException: Wrong FS:
> file:/Users/stack/checkouts/hbase.apache.git/hbase-mapreduce/target/test-data/fad01338-65ef-e973-2412-05de114016fe/.hbase-snapshot/tableWithRefsV2,
> expected: hdfs://localhost:58051
>at
> org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotWithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:107)
>at
> org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:91)
> {code}
> With v1 had just run find which does same thing.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (HBASE-23824) TestSnapshotScannerHDFSAclController is flakey
[
https://issues.apache.org/jira/browse/HBASE-23824?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033941#comment-17033941
]
Hudson commented on HBASE-23824:
Results for branch branch-2
[build #2464 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//JDK8_Nightly_Build_Report_(Hadoop2)/]
(/) {color:green}+1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2464//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> TestSnapshotScannerHDFSAclController is flakey
> --
>
> Key: HBASE-23824
> URL: https://issues.apache.org/jira/browse/HBASE-23824
> Project: HBase
> Issue Type: Bug
>Reporter: Yi Mei
>Assignee: Yi Mei
>Priority: Major
> Fix For: 3.0.0, 2.3.0
>
>
> See
> [https://builds.apache.org/view/H-L/view/HBase/job/HBase-Find-Flaky-Tests/job/branch-2/lastSuccessfulBuild/artifact/dashboard.html]
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377314093
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
##
@@ -183,105 +271,184 @@ protected boolean performCompaction(FileDetails fd,
InternalScanner scanner, Cel
boolean hasMore;
Path path = MobUtils.getMobFamilyPath(conf, store.getTableName(),
store.getColumnFamilyName());
byte[] fileName = null;
-StoreFileWriter mobFileWriter = null, delFileWriter = null;
-long mobCells = 0, deleteMarkersCount = 0;
+StoreFileWriter mobFileWriter = null;
+/*
+ * mobCells are used only to decide if we need to commit or abort current
MOB output file.
+ */
+long mobCells = 0;
long cellsCountCompactedToMob = 0, cellsCountCompactedFromMob = 0;
long cellsSizeCompactedToMob = 0, cellsSizeCompactedFromMob = 0;
boolean finished = false;
+
ScannerContext scannerContext =
ScannerContext.newBuilder().setBatchLimit(compactionKVMax).build();
throughputController.start(compactionName);
-KeyValueScanner kvs = (scanner instanceof KeyValueScanner)?
(KeyValueScanner)scanner : null;
-long shippedCallSizeLimit = (long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+KeyValueScanner kvs = (scanner instanceof KeyValueScanner) ?
(KeyValueScanner) scanner : null;
+long shippedCallSizeLimit =
+(long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+
+Cell mobCell = null;
try {
- try {
-// If the mob file writer could not be created, directly write the
cell to the store file.
-mobFileWriter = mobStore.createWriterInTmp(new Date(fd.latestPutTs),
fd.maxKeyCount,
- compactionCompression, store.getRegionInfo().getStartKey(), true);
-fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
- } catch (IOException e) {
-LOG.warn("Failed to create mob writer, "
- + "we will continue the compaction by writing MOB cells
directly in store files", e);
- }
- if (major) {
-try {
- delFileWriter = mobStore.createDelFileWriterInTmp(new
Date(fd.latestPutTs),
-fd.maxKeyCount, compactionCompression,
store.getRegionInfo().getStartKey());
-} catch (IOException e) {
- LOG.warn(
-"Failed to create del writer, "
-+ "we will continue the compaction by writing delete markers
directly in store files",
-e);
-}
- }
+
+ mobFileWriter = newMobWriter(fd);
+ fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
+
do {
hasMore = scanner.next(cells, scannerContext);
-if (LOG.isDebugEnabled()) {
- now = EnvironmentEdgeManager.currentTime();
-}
+now = EnvironmentEdgeManager.currentTime();
for (Cell c : cells) {
- if (major && CellUtil.isDelete(c)) {
-if (MobUtils.isMobReferenceCell(c) || delFileWriter == null) {
- // Directly write it to a store file
- writer.append(c);
+ if (compactMOBs) {
+if (MobUtils.isMobReferenceCell(c)) {
+ String fName = MobUtils.getMobFileName(c);
+ Path pp = new Path(new Path(fs.getUri()), new Path(path, fName));
+
+ // Added to support migration
+ try {
+mobCell = mobStore.resolve(c, true, false).getCell();
+ } catch (FileNotFoundException fnfe) {
+if (discardMobMiss) {
+ LOG.error("Missing MOB cell: file={} not found cell={}",
fName, c);
+ continue;
+} else {
+ throw fnfe;
+}
+ }
+
+ if (discardMobMiss && mobCell.getValueLength() == 0) {
+LOG.error("Missing MOB cell value: file={} cell={}", pp,
mobCell);
+continue;
+ } else if (mobCell.getValueLength() == 0) {
+String errMsg = String.format("Found 0 length MOB cell in a
file=%s cell=%s",
Review comment:
ok
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on issue #1143: Backport HBASE-15519, HBASE-23364, HBASE-23802 to branch-2.2
ndimiduk commented on issue #1143: Backport HBASE-15519, HBASE-23364, HBASE-23802 to branch-2.2 URL: https://github.com/apache/hbase/pull/1143#issuecomment-584365689 I think these additional back ports don't make sense for branch-2.2. Closing this one out in favor a of a simplified patch that addresses only the issue as described in the JIRA that's being back ported. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] ndimiduk closed pull request #1143: Backport HBASE-15519, HBASE-23364, HBASE-23802 to branch-2.2
ndimiduk closed pull request #1143: Backport HBASE-15519, HBASE-23364, HBASE-23802 to branch-2.2 URL: https://github.com/apache/hbase/pull/1143 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Created] (HBASE-23825) Increment proto conversion is broken
Abhishek Singh Chouhan created HBASE-23825: -- Summary: Increment proto conversion is broken Key: HBASE-23825 URL: https://issues.apache.org/jira/browse/HBASE-23825 Project: HBase Issue Type: Bug Affects Versions: 1.4.12, 1.3.6, 1.5.0 Reporter: Abhishek Singh Chouhan Assignee: Abhishek Singh Chouhan While converting the request back to Increment using ProtobufUtil.toIncrement we incorrectly use the optimization to avoid copying the byte array(HBaseZeroCopyByteString#zeroCopyGetBytes) on a BoundedByteString. The optimization was only meant for LiteralByteString where it is safe to use the backing byte array, however it ends up being used to BoundedByteString which is a subclass of LiteralByteString. This essentially breaks increments since we end up creating wrong cells on the server side. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-23744) FastPathBalancedQueueRpcExecutor should enforce queue length of 0
[ https://issues.apache.org/jira/browse/HBASE-23744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033872#comment-17033872 ] Geoffrey Jacoby commented on HBASE-23744: - [~anoop.hbase][~xucang][~ndimiduk] - as far as I know, the "shut down writes by setting the call queue length to 0" behavior is undocumented. And I agree with Xu's point that it's a very subtle, less obvious way to do it. It is, however, tested for in TestSimpleRpcScehduler.testSoftAndHardQueueLimits:451-4. This test just happens to pass because in practice the FastPathBalancedQueueRpcExecutor is always getting lucky and never has a fast path handler in the stack when the test is run. I came across the bug because an internal fork of HBase I use happened to not always be lucky. An alternative fix would be to declare the 0 call queue behavior undefined and remove the 0 test logic from testSoftAndHardQueueLimits, but I don't think it's a good idea to leave the test logic as-is without the fix in this patch making the Fast executor work similarly to the other executor types. And regardless of which path we take in this JIRA, a supported, well-documented kill switch for writes is definitely something that I as an operator have wished for before. :-) (For example, to block all writes before running a risky hbck operation.) > FastPathBalancedQueueRpcExecutor should enforce queue length of 0 > - > > Key: HBASE-23744 > URL: https://issues.apache.org/jira/browse/HBASE-23744 > Project: HBase > Issue Type: Bug >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Minor > > FastPathBalancedQueueRpcExecutor allows RPC requests to skip the RPC queue > and get worked by an available handler under certain circumstances. > Relatedly, the hbase.ipc.server.max.callqueue.length parameter can be set to > 0, including dynamically. This can be useful to temporarily prevent writes on > a cluster. > When this is the case the executor is supposed to block all dispatching. > However, the FastPathBalancedQueueRpcExecutor will still dispatch the request > if one of the "fast path" handlers is available on its stack. This both isn't > the desired effect, and also makes > TestSimpleRpcScheduler.testSoftAndHardQueueLimits unstable when it checks the > queue length 0 behavior. > A simple fix is just to check max queue length > 0 before > FastPathBalancedQueueRpcExecutor pops the fast handler off the stack. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-23779) Up the default fork count to make builds complete faster; make count relative to CPU count
[ https://issues.apache.org/jira/browse/HBASE-23779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033905#comment-17033905 ] Michael Stack commented on HBASE-23779: --- bq. I don't think it is always around 5k. You are right. I see 9k+. Let me try [~markrmiller] suggested trick in subtask. Yetus polling probably misses spikes? On native thread creation, my experience has been that not enough file handles can manifest in mysterious ways. Can work on other Mark suggestions in follow-ons. Heading down to the subtask now... > Up the default fork count to make builds complete faster; make count relative > to CPU count > -- > > Key: HBASE-23779 > URL: https://issues.apache.org/jira/browse/HBASE-23779 > Project: HBase > Issue Type: Bug > Components: test >Reporter: Michael Stack >Assignee: Michael Stack >Priority: Major > Fix For: 3.0.0, 2.3.0 > > Attachments: addendum2.patch, test_yetus_934.0.patch > > > Tests take a long time. Our fork count running all tests are conservative -- > 1 (small) for first part and 5 for second part (medium and large). Rather > than hardcoding we should set the fork count to be relative to machine size. > Suggestion here is 0.75C where C is CPU count. This ups the CPU use on my box. > Looking up at jenkins, it seems like the boxes are 24 cores... at least going > by my random survey. The load reported on a few seems low though this not > representative (looking at machine/uptime). > More parallelism willl probably mean more test failure. Let me take a look > see. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [hbase] busbey commented on a change in pull request #1158: HBASE-23823 Run maven with more than default single thread (--threads…
busbey commented on a change in pull request #1158: HBASE-23823 Run maven with more than default single thread (--threads… URL: https://github.com/apache/hbase/pull/1158#discussion_r377288653 ## File path: dev-support/hbase-personality.sh ## @@ -140,7 +140,11 @@ function personality_modules clear_personality_queue - extra="-DHBasePatchProcess" + # Pass maven a -T argument. Should make it run faster. Pass conservative value. + # Default is one thread. 0.5C on an apache box of 24 cores and 2 executors should + # make for 6 threads? Lets see. See below for more on -T: Review comment: it'll be 12 I think. AFAIK each executor believes there are 24 cores on the box. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] busbey commented on a change in pull request #1158: HBASE-23823 Run maven with more than default single thread (--threads…
busbey commented on a change in pull request #1158: HBASE-23823 Run maven with
more than default single thread (--threads…
URL: https://github.com/apache/hbase/pull/1158#discussion_r377290392
##
File path: dev-support/jenkins-scripts/generate-hbase-website.sh
##
@@ -120,6 +120,7 @@ if [ -z "${MAVEN_HOME}" ]; then
export PATH="${MAVEN_HOME}/bin:${PATH}"
fi
export MAVEN_OPTS="${MAVEN_OPTS} -Dmaven.repo.local=${local_repo}"
+export MAVEN_ARGS="--threads=0.5C ${MAVEN_ARGS}"
Review comment:
trying to optimize the website build at the same time as the precommit /
nightly tests feels overloaded to me. How are we going to test this 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] busbey commented on a change in pull request #1158: HBASE-23823 Run maven with more than default single thread (--threads…
busbey commented on a change in pull request #1158: HBASE-23823 Run maven with
more than default single thread (--threads…
URL: https://github.com/apache/hbase/pull/1158#discussion_r377289534
##
File path: pom.xml
##
@@ -1428,7 +1428,7 @@
-Dorg.apache.hbase.thirdparty.io.netty.leakDetection.level=advanced
-${hbase-surefire.argLine}
+-XX:-MaxFDLimit ${hbase-surefire.argLine}
Review comment:
please include a comment about why we're adding this option.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377310286
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
##
@@ -183,105 +271,184 @@ protected boolean performCompaction(FileDetails fd,
InternalScanner scanner, Cel
boolean hasMore;
Path path = MobUtils.getMobFamilyPath(conf, store.getTableName(),
store.getColumnFamilyName());
byte[] fileName = null;
-StoreFileWriter mobFileWriter = null, delFileWriter = null;
-long mobCells = 0, deleteMarkersCount = 0;
+StoreFileWriter mobFileWriter = null;
+/*
+ * mobCells are used only to decide if we need to commit or abort current
MOB output file.
+ */
+long mobCells = 0;
long cellsCountCompactedToMob = 0, cellsCountCompactedFromMob = 0;
long cellsSizeCompactedToMob = 0, cellsSizeCompactedFromMob = 0;
boolean finished = false;
+
ScannerContext scannerContext =
ScannerContext.newBuilder().setBatchLimit(compactionKVMax).build();
throughputController.start(compactionName);
-KeyValueScanner kvs = (scanner instanceof KeyValueScanner)?
(KeyValueScanner)scanner : null;
-long shippedCallSizeLimit = (long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+KeyValueScanner kvs = (scanner instanceof KeyValueScanner) ?
(KeyValueScanner) scanner : null;
+long shippedCallSizeLimit =
+(long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+
+Cell mobCell = null;
try {
- try {
-// If the mob file writer could not be created, directly write the
cell to the store file.
-mobFileWriter = mobStore.createWriterInTmp(new Date(fd.latestPutTs),
fd.maxKeyCount,
- compactionCompression, store.getRegionInfo().getStartKey(), true);
-fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
- } catch (IOException e) {
-LOG.warn("Failed to create mob writer, "
- + "we will continue the compaction by writing MOB cells
directly in store files", e);
- }
- if (major) {
-try {
- delFileWriter = mobStore.createDelFileWriterInTmp(new
Date(fd.latestPutTs),
-fd.maxKeyCount, compactionCompression,
store.getRegionInfo().getStartKey());
-} catch (IOException e) {
- LOG.warn(
-"Failed to create del writer, "
-+ "we will continue the compaction by writing delete markers
directly in store files",
-e);
-}
- }
+
+ mobFileWriter = newMobWriter(fd);
+ fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
+
do {
hasMore = scanner.next(cells, scannerContext);
-if (LOG.isDebugEnabled()) {
- now = EnvironmentEdgeManager.currentTime();
-}
+now = EnvironmentEdgeManager.currentTime();
for (Cell c : cells) {
- if (major && CellUtil.isDelete(c)) {
-if (MobUtils.isMobReferenceCell(c) || delFileWriter == null) {
- // Directly write it to a store file
- writer.append(c);
+ if (compactMOBs) {
+if (MobUtils.isMobReferenceCell(c)) {
+ String fName = MobUtils.getMobFileName(c);
+ Path pp = new Path(new Path(fs.getUri()), new Path(path, fName));
+
+ // Added to support migration
+ try {
+mobCell = mobStore.resolve(c, true, false).getCell();
+ } catch (FileNotFoundException fnfe) {
+if (discardMobMiss) {
+ LOG.error("Missing MOB cell: file={} not found cell={}",
fName, c);
+ continue;
+} else {
+ throw fnfe;
+}
+ }
+
+ if (discardMobMiss && mobCell.getValueLength() == 0) {
+LOG.error("Missing MOB cell value: file={} cell={}", pp,
mobCell);
Review comment:
That was fixed in the last 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk opened a new pull request #1159: HBASE-23802 Remove unnecessary Configuration instantiation in LossyAc…
ndimiduk opened a new pull request #1159: HBASE-23802 Remove unnecessary Configuration instantiation in LossyAc… URL: https://github.com/apache/hbase/pull/1159 …counting (#1127) Signed-off-by: stack 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] ndimiduk commented on issue #1159: HBASE-23802 Remove unnecessary Configuration instantiation in LossyAc…
ndimiduk commented on issue #1159: HBASE-23802 Remove unnecessary Configuration instantiation in LossyAc… URL: https://github.com/apache/hbase/pull/1159#issuecomment-584368355 This PR replaces #1143. 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Reopened] (HBASE-23816) [Flakey Test] TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:91) Wrong FS!
[
https://issues.apache.org/jira/browse/HBASE-23816?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Michael Stack reopened HBASE-23816:
---
Reopening. I just got this in a local test run. Same old failure type:
{code}
[ERROR]
org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState
Time elapsed: 93.988 s <<< ERROR!
java.lang.IllegalArgumentException: Wrong FS:
file:/Users/stack/checkouts/hbase.apache.git/hbase-mapreduce/target/test-data/bbeebf8f-21cd-f704-4f81-5f6dd7fa1c38/.hbase-snapshot/tableWithRefsV2,
expected: hdfs://localhost:61762
at
org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotWithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:108)
at
org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:92)
{code}
> [Flakey Test]
> TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:91)
> Wrong FS!
>
>
> Key: HBASE-23816
> URL: https://issues.apache.org/jira/browse/HBASE-23816
> Project: HBase
> Issue Type: Sub-task
>Reporter: Michael Stack
>Assignee: Michael Stack
>Priority: Major
> Fix For: 3.0.0, 2.3.0
>
>
> Can't repro locally w/o running with lots of parallelism. Follow-on from
> parent issue.
> Here is error:
> {code}
> [ERROR] Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed:
> 115.741 s <<< FAILURE! - in
> org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster
> [ERROR]
> org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState
> Time elapsed: 74.459 s <<< ERROR!
> java.lang.IllegalArgumentException: Wrong FS:
> file:/Users/stack/checkouts/hbase.apache.git/hbase-mapreduce/target/test-data/fad01338-65ef-e973-2412-05de114016fe/.hbase-snapshot/tableWithRefsV2,
> expected: hdfs://localhost:58051
>at
> org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotWithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:107)
>at
> org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:91)
> {code}
> With v1 had just run find which does same thing.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [hbase] busbey commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
busbey commented on a change in pull request #921: HBASE-22749: Distributed MOB
compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377345640
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCleanerChore.java
##
@@ -0,0 +1,331 @@
+/**
+ * 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.mob;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.apache.hadoop.hbase.ScheduledChore;
+import org.apache.hadoop.hbase.TableDescriptors;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.backup.HFileArchiver;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.regionserver.BloomType;
+import org.apache.hadoop.hbase.regionserver.HStoreFile;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.hadoop.hbase.util.FSUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+
+/**
+ * The class MobFileCleanerChore for running cleaner regularly to remove the
expired
+ * and obsolete (files which have no active references to) mob files.
+ */
+@InterfaceAudience.Private
+public class MobFileCleanerChore extends ScheduledChore {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MobFileCleanerChore.class);
+ private final HMaster master;
+ private ExpiredMobFileCleaner cleaner;
+
+ static {
+Configuration.addDeprecation(MobConstants.DEPRECATED_MOB_CLEANER_PERIOD,
+ MobConstants.MOB_CLEANER_PERIOD);
+ }
+
+ public MobFileCleanerChore(HMaster master) {
+super(master.getServerName() + "-ExpiredMobFileCleanerChore", master,
+master.getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD,
+ MobConstants.DEFAULT_MOB_CLEANER_PERIOD),
+master.getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD,
+ MobConstants.DEFAULT_MOB_CLEANER_PERIOD),
+TimeUnit.SECONDS);
+this.master = master;
+cleaner = new ExpiredMobFileCleaner();
+cleaner.setConf(master.getConfiguration());
+checkObsoleteConfigurations();
+ }
+
+ private void checkObsoleteConfigurations() {
+Configuration conf = master.getConfiguration();
+
+if (conf.get("hbase.mob.compaction.mergeable.threshold") != null) {
+ LOG.warn("'hbase.mob.compaction.mergeable.threshold' is obsolete and not
used anymore.");
+}
+if (conf.get("hbase.mob.delfile.max.count") != null) {
+ LOG.warn("'hbase.mob.delfile.max.count' is obsolete and not used
anymore.");
+}
+if (conf.get("hbase.mob.compaction.threads.max") != null) {
+ LOG.warn("'hbase.mob.compaction.threads.max' is obsolete and not used
anymore.");
+}
+if (conf.get("hbase.mob.compaction.batch.size") != null) {
+ LOG.warn("'hbase.mob.compaction.batch.size' is obsolete and not used
anymore.");
+}
+ }
+
+ @VisibleForTesting
+ public MobFileCleanerChore() {
+this.master = null;
+ }
+
+ @Override
+ @edu.umd.cs.findbugs.annotations.SuppressWarnings(value =
"REC_CATCH_EXCEPTION",
+ justification = "Intentional")
+
+ protected void chore() {
+TableDescriptors htds = master.getTableDescriptors();
+
+Map map = null;
+try {
+ map = htds.getAll();
+} catch (IOException e) {
+
[GitHub] [hbase] busbey commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
busbey commented on a change in pull request #921: HBASE-22749: Distributed MOB
compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377358650
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCleanerChore.java
##
@@ -0,0 +1,331 @@
+/**
+ * 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.mob;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.apache.hadoop.hbase.ScheduledChore;
+import org.apache.hadoop.hbase.TableDescriptors;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.backup.HFileArchiver;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.regionserver.BloomType;
+import org.apache.hadoop.hbase.regionserver.HStoreFile;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.hadoop.hbase.util.FSUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+
+/**
+ * The class MobFileCleanerChore for running cleaner regularly to remove the
expired
+ * and obsolete (files which have no active references to) mob files.
+ */
+@InterfaceAudience.Private
+public class MobFileCleanerChore extends ScheduledChore {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MobFileCleanerChore.class);
+ private final HMaster master;
+ private ExpiredMobFileCleaner cleaner;
+
+ static {
+Configuration.addDeprecation(MobConstants.DEPRECATED_MOB_CLEANER_PERIOD,
+ MobConstants.MOB_CLEANER_PERIOD);
+ }
+
+ public MobFileCleanerChore(HMaster master) {
+super(master.getServerName() + "-ExpiredMobFileCleanerChore", master,
+master.getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD,
+ MobConstants.DEFAULT_MOB_CLEANER_PERIOD),
+master.getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD,
+ MobConstants.DEFAULT_MOB_CLEANER_PERIOD),
+TimeUnit.SECONDS);
+this.master = master;
+cleaner = new ExpiredMobFileCleaner();
+cleaner.setConf(master.getConfiguration());
+checkObsoleteConfigurations();
+ }
+
+ private void checkObsoleteConfigurations() {
+Configuration conf = master.getConfiguration();
+
+if (conf.get("hbase.mob.compaction.mergeable.threshold") != null) {
+ LOG.warn("'hbase.mob.compaction.mergeable.threshold' is obsolete and not
used anymore.");
+}
+if (conf.get("hbase.mob.delfile.max.count") != null) {
+ LOG.warn("'hbase.mob.delfile.max.count' is obsolete and not used
anymore.");
+}
+if (conf.get("hbase.mob.compaction.threads.max") != null) {
+ LOG.warn("'hbase.mob.compaction.threads.max' is obsolete and not used
anymore.");
+}
+if (conf.get("hbase.mob.compaction.batch.size") != null) {
+ LOG.warn("'hbase.mob.compaction.batch.size' is obsolete and not used
anymore.");
+}
+ }
+
+ @VisibleForTesting
+ public MobFileCleanerChore() {
+this.master = null;
+ }
+
+ @Override
+ @edu.umd.cs.findbugs.annotations.SuppressWarnings(value =
"REC_CATCH_EXCEPTION",
+ justification = "Intentional")
+
+ protected void chore() {
+TableDescriptors htds = master.getTableDescriptors();
+
+Map map = null;
+try {
+ map = htds.getAll();
+} catch (IOException e) {
+
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377363508
##
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestMobCompactionOptRegionBatchMode.java
##
@@ -0,0 +1,98 @@
+/**
+ *
+ * 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.mob;
+import java.io.IOException;
+
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HColumnDescriptor;
+import org.apache.hadoop.hbase.HTableDescriptor;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.testclassification.LargeTests;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Mob file compaction chore in a generational batch mode test.
+ * 1. Enables batch mode for regular MOB compaction,
+ *Sets batch size to 7 regions. Enables generational mode.
+ * 2. Disables periodic MOB compactions, sets minimum age to archive to 10 sec
+ * 3. Creates MOB table with 20 regions
+ * 4. Loads MOB data (randomized keys, 1000 rows), flushes data.
+ * 5. Repeats 4. two more times
+ * 6. Verifies that we have 20 *3 = 60 mob files (equals to number of regions
x 3)
+ * 7. Runs major MOB compaction.
+ * 8. Verifies that number of MOB files in a mob directory is 20 x4 = 80
+ * 9. Waits for a period of time larger than minimum age to archive
+ * 10. Runs Mob cleaner chore
+ * 11 Verifies that number of MOB files in a mob directory is 20.
+ * 12 Runs scanner and checks all 3 * 1000 rows.
+ */
+@SuppressWarnings("deprecation")
+@Category(LargeTests.class)
+public class TestMobCompactionOptRegionBatchMode extends TestMobCompactionBase{
+ private static final Logger LOG =
+ LoggerFactory.getLogger(TestMobCompactionOptRegionBatchMode.class);
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestMobCompactionOptRegionBatchMode.class);
+ @Rule
+ public TestName testName = new TestName();
Review comment:
It defines test timeout according to a category of test (small, medium,
large).
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:
us...@infra.apache.org
With regards,
Apache Git Services
[jira] [Commented] (HBASE-23822) Fix typo in procedures.jsp
[
https://issues.apache.org/jira/browse/HBASE-23822?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034035#comment-17034035
]
Hudson commented on HBASE-23822:
Results for branch branch-2.2
[build #781 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.2/781/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.2/781//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.2/781//JDK8_Nightly_Build_Report_(Hadoop2)/]
(x) {color:red}-1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.2/781//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> Fix typo in procedures.jsp
> --
>
> Key: HBASE-23822
> URL: https://issues.apache.org/jira/browse/HBASE-23822
> Project: HBase
> Issue Type: Improvement
> Components: website
>Affects Versions: master
>Reporter: Zhuoyue Huang
>Assignee: Zhuoyue Huang
>Priority: Trivial
> Fix For: 3.0.0, 2.3.0, 2.2.4
>
> Attachments: procedures.jsp
>
>
> I think there's a typo.In
> hbase-server/src/main/resources/hbase-webapps/master/procedures.jsp. The
> Sentences "We do not list Procedures that have completed SUCCESSfully; their
> number makes it hard to spot the problematics." are misspelled and need to be
> corrected as "We do not list procedures that have completed successfully;
> their number makes it hard to spot the problematics."
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [hbase] saintstack opened a new pull request #1162: HBASE-23816 [Flakey Test] TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:91) Wron
saintstack opened a new pull request #1162: HBASE-23816 [Flakey Test] TestExportSnapshotNoCluster.testSnapshotV2WithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:91) Wrong FS! URL: https://github.com/apache/hbase/pull/1162 ADDENDUM: Break test in two to see if it makes it more reliable. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #1161: HBASE-23825 Increment proto conversion is broken
Apache-HBase commented on issue #1161: HBASE-23825 Increment proto conversion is broken URL: https://github.com/apache/hbase/pull/1161#issuecomment-584422591 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 0m 48s | Docker mode activated. | ||| _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. | | +1 :green_heart: | hbaseanti | 0m 1s | Patch does not have any anti-patterns. | | +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. | | +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. | ||| _ branch-1 Compile Tests _ | | +0 :ok: | mvndep | 1m 26s | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 7m 33s | branch-1 passed | | +1 :green_heart: | compile | 1m 10s | branch-1 passed with JDK v1.8.0_242 | | +1 :green_heart: | compile | 1m 16s | branch-1 passed with JDK v1.7.0_252 | | +1 :green_heart: | checkstyle | 4m 5s | branch-1 passed | | +1 :green_heart: | shadedjars | 2m 55s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 0m 49s | branch-1 passed with JDK v1.8.0_242 | | +1 :green_heart: | javadoc | 0m 53s | branch-1 passed with JDK v1.7.0_252 | | +0 :ok: | spotbugs | 2m 41s | Used deprecated FindBugs config; considering switching to SpotBugs. | | +1 :green_heart: | findbugs | 4m 56s | branch-1 passed | ||| _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 17s | Maven dependency ordering for patch | | -1 :x: | mvninstall | 1m 13s | root in the patch failed. | | +1 :green_heart: | compile | 1m 2s | the patch passed with JDK v1.8.0_242 | | +1 :green_heart: | javac | 1m 2s | the patch passed | | -1 :x: | compile | 0m 48s | hbase-server in the patch failed with JDK v1.7.0_252. | | -1 :x: | javac | 0m 48s | hbase-server in the patch failed with JDK v1.7.0_252. | | -1 :x: | checkstyle | 1m 29s | hbase-server: The patch generated 1 new + 8 unchanged - 1 fixed = 9 total (was 9) | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | -1 :x: | shadedjars | 1m 56s | patch has 16 errors when building our shaded downstream artifacts. | | -1 :x: | hadoopcheck | 1m 8s | The patch causes 16 errors with Hadoop v2.8.5. | | -1 :x: | hadoopcheck | 3m 12s | The patch causes 16 errors with Hadoop v2.9.2. | | +1 :green_heart: | javadoc | 0m 41s | the patch passed with JDK v1.8.0_242 | | +1 :green_heart: | javadoc | 0m 53s | the patch passed with JDK v1.7.0_252 | | -1 :x: | findbugs | 0m 42s | hbase-server in the patch failed. | ||| _ Other Tests _ | | +1 :green_heart: | unit | 0m 28s | hbase-protocol in the patch passed. | | -1 :x: | unit | 0m 43s | hbase-server in the patch failed. | | +1 :green_heart: | asflicense | 0m 25s | The patch does not generate ASF License warnings. | | | | 46m 46s | | | Subsystem | Report/Notes | |--:|:-| | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1161/1/artifact/out/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/1161 | | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile | | uname | Linux e6f1128f058d 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 | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1161/out/precommit/personality/provided.sh | | git revision | branch-1 / 942bb77 | | Default Java | 1.7.0_252 | | Multi-JDK versions | /usr/lib/jvm/zulu-8-amd64:1.8.0_242 /usr/lib/jvm/zulu-7-amd64:1.7.0_252 | | mvninstall | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1161/1/artifact/out/patch-mvninstall-root.txt | | compile | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1161/1/artifact/out/patch-compile-hbase-server-jdk1.7.0_252.txt | | javac | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1161/1/artifact/out/patch-compile-hbase-server-jdk1.7.0_252.txt | | checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1161/1/artifact/out/diff-checkstyle-hbase-server.txt | | shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1161/1/artifact/out/patch-shadedjars.txt | | hadoopcheck | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1161/1/artifact/out/patch-javac-2.8.5.txt | |
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377392972
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
##
@@ -183,105 +271,184 @@ protected boolean performCompaction(FileDetails fd,
InternalScanner scanner, Cel
boolean hasMore;
Path path = MobUtils.getMobFamilyPath(conf, store.getTableName(),
store.getColumnFamilyName());
byte[] fileName = null;
-StoreFileWriter mobFileWriter = null, delFileWriter = null;
-long mobCells = 0, deleteMarkersCount = 0;
+StoreFileWriter mobFileWriter = null;
+/*
+ * mobCells are used only to decide if we need to commit or abort current
MOB output file.
+ */
+long mobCells = 0;
long cellsCountCompactedToMob = 0, cellsCountCompactedFromMob = 0;
long cellsSizeCompactedToMob = 0, cellsSizeCompactedFromMob = 0;
boolean finished = false;
+
ScannerContext scannerContext =
ScannerContext.newBuilder().setBatchLimit(compactionKVMax).build();
throughputController.start(compactionName);
-KeyValueScanner kvs = (scanner instanceof KeyValueScanner)?
(KeyValueScanner)scanner : null;
-long shippedCallSizeLimit = (long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+KeyValueScanner kvs = (scanner instanceof KeyValueScanner) ?
(KeyValueScanner) scanner : null;
+long shippedCallSizeLimit =
+(long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+
+Cell mobCell = null;
try {
- try {
-// If the mob file writer could not be created, directly write the
cell to the store file.
-mobFileWriter = mobStore.createWriterInTmp(new Date(fd.latestPutTs),
fd.maxKeyCount,
- compactionCompression, store.getRegionInfo().getStartKey(), true);
-fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
- } catch (IOException e) {
-LOG.warn("Failed to create mob writer, "
- + "we will continue the compaction by writing MOB cells
directly in store files", e);
- }
- if (major) {
-try {
- delFileWriter = mobStore.createDelFileWriterInTmp(new
Date(fd.latestPutTs),
-fd.maxKeyCount, compactionCompression,
store.getRegionInfo().getStartKey());
-} catch (IOException e) {
- LOG.warn(
-"Failed to create del writer, "
-+ "we will continue the compaction by writing delete markers
directly in store files",
-e);
-}
- }
+
+ mobFileWriter = newMobWriter(fd);
+ fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
+
do {
hasMore = scanner.next(cells, scannerContext);
-if (LOG.isDebugEnabled()) {
- now = EnvironmentEdgeManager.currentTime();
-}
+now = EnvironmentEdgeManager.currentTime();
for (Cell c : cells) {
- if (major && CellUtil.isDelete(c)) {
-if (MobUtils.isMobReferenceCell(c) || delFileWriter == null) {
- // Directly write it to a store file
- writer.append(c);
+ if (compactMOBs) {
+if (MobUtils.isMobReferenceCell(c)) {
+ String fName = MobUtils.getMobFileName(c);
+ Path pp = new Path(new Path(fs.getUri()), new Path(path, fName));
+
+ // Added to support migration
+ try {
+mobCell = mobStore.resolve(c, true, false).getCell();
+ } catch (FileNotFoundException fnfe) {
+if (discardMobMiss) {
+ LOG.error("Missing MOB cell: file={} not found cell={}",
fName, c);
+ continue;
+} else {
+ throw fnfe;
+}
+ }
+
+ if (discardMobMiss && mobCell.getValueLength() == 0) {
+LOG.error("Missing MOB cell value: file={} cell={}", pp,
mobCell);
+continue;
+ } else if (mobCell.getValueLength() == 0) {
+String errMsg = String.format("Found 0 length MOB cell in a
file=%s cell=%s",
+ fName, mobCell);
+throw new IOException(errMsg);
+ }
+
+ if (mobCell.getValueLength() > mobSizeThreshold) {
+// put the mob data back to the MOB store file
+PrivateCellUtil.setSequenceId(mobCell, c.getSequenceId());
+if (!ioOptimizedMode) {
+ mobFileWriter.append(mobCell);
+ mobCells++;
+ writer.append(
+MobUtils.createMobRefCell(mobCell, fileName,
this.mobStore.getRefCellTags()));
+} else {
+ // I/O optimized mode
+ // Check if MOB cell origin file size is
+ // greater than threshold
+
[jira] [Commented] (HBASE-23744) FastPathBalancedQueueRpcExecutor should enforce queue length of 0
[ https://issues.apache.org/jira/browse/HBASE-23744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033967#comment-17033967 ] Nick Dimiduk commented on HBASE-23744: -- bq. And regardless of which path we take in this JIRA, a supported, well-documented kill switch for writes is definitely something that I as an operator have wished for before. How about filing a ticket requesting a read-only mode? You can outline exactly what you're after there, we can has out requirements, > FastPathBalancedQueueRpcExecutor should enforce queue length of 0 > - > > Key: HBASE-23744 > URL: https://issues.apache.org/jira/browse/HBASE-23744 > Project: HBase > Issue Type: Bug >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Minor > > FastPathBalancedQueueRpcExecutor allows RPC requests to skip the RPC queue > and get worked by an available handler under certain circumstances. > Relatedly, the hbase.ipc.server.max.callqueue.length parameter can be set to > 0, including dynamically. This can be useful to temporarily prevent writes on > a cluster. > When this is the case the executor is supposed to block all dispatching. > However, the FastPathBalancedQueueRpcExecutor will still dispatch the request > if one of the "fast path" handlers is available on its stack. This both isn't > the desired effect, and also makes > TestSimpleRpcScheduler.testSoftAndHardQueueLimits unstable when it checks the > queue length 0 behavior. > A simple fix is just to check max queue length > 0 before > FastPathBalancedQueueRpcExecutor pops the fast handler off the stack. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [hbase] bharathv closed pull request #1101: HBASE-23305: Master based registry implementation (#954)
bharathv closed pull request #1101: HBASE-23305: Master based registry implementation (#954) URL: https://github.com/apache/hbase/pull/1101 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] apurtell commented on issue #1101: HBASE-23305: Master based registry implementation (#954)
apurtell commented on issue #1101: HBASE-23305: Master based registry implementation (#954) URL: https://github.com/apache/hbase/pull/1101#issuecomment-584389104 Thank 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Commented] (HBASE-23823) Run maven with more than default single thread (--threads=1C)
[ https://issues.apache.org/jira/browse/HBASE-23823?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033917#comment-17033917 ] Sean Busbey commented on HBASE-23823: - to check how heavy we're hitting the thread limits, I'd recommend we only test this change w/ something that touches the root pom. That should ensure we do a full build of all modules. > Run maven with more than default single thread (--threads=1C) > - > > Key: HBASE-23823 > URL: https://issues.apache.org/jira/browse/HBASE-23823 > Project: HBase > Issue Type: Sub-task >Reporter: Michael Stack >Priority: Major > > See tail of parent task where we made commits to hbase-personality trying to > make maven run more furiously on nightly builds by upping the maven thread > count (the --threads flag ... see > https://cwiki.apache.org/confluence/display/MAVEN/Parallel+builds+in+Maven+3). > The effort was abandoned because it unsettled the build. This issue > separates messing with the maven threads flag from the broader topic the > parent covers. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [hbase] saintstack commented on a change in pull request #1158: HBASE-23823 Run maven with more than default single thread (--threads…
saintstack commented on a change in pull request #1158: HBASE-23823 Run maven with more than default single thread (--threads… URL: https://github.com/apache/hbase/pull/1158#discussion_r377292283 ## File path: dev-support/hbase-personality.sh ## @@ -140,7 +140,11 @@ function personality_modules clear_personality_queue - extra="-DHBasePatchProcess" + # Pass maven a -T argument. Should make it run faster. Pass conservative value. + # Default is one thread. 0.5C on an apache box of 24 cores and 2 executors should + # make for 6 threads? Lets see. See below for more on -T: Review comment: 0.5 * 12 = 6. Let me change the title. It doesn't agree w/ what is in the patch which is trying to keep us close to current settings. Thanks @busbey 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] saintstack commented on a change in pull request #1158: HBASE-23823 Run maven with more than default single thread (--threads…
saintstack commented on a change in pull request #1158: HBASE-23823 Run maven
with more than default single thread (--threads…
URL: https://github.com/apache/hbase/pull/1158#discussion_r377292863
##
File path: dev-support/jenkins-scripts/generate-hbase-website.sh
##
@@ -120,6 +120,7 @@ if [ -z "${MAVEN_HOME}" ]; then
export PATH="${MAVEN_HOME}/bin:${PATH}"
fi
export MAVEN_OPTS="${MAVEN_OPTS} -Dmaven.repo.local=${local_repo}"
+export MAVEN_ARGS="--threads=0.5C ${MAVEN_ARGS}"
Review comment:
Was just trying to cover all the mvn references. Let me drop this for now.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] apurtell commented on issue #1101: HBASE-23305: Master based registry implementation (#954)
apurtell commented on issue #1101: HBASE-23305: Master based registry implementation (#954) URL: https://github.com/apache/hbase/pull/1101#issuecomment-584387917 Is this the consolidated branch-2 backport? /cc @ndimiduk @saintstack It looks to be with 114 changed files, just want to confirm. 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Commented] (HBASE-18095) Provide an option for clients to find the server hosting META that does not involve the ZooKeeper client
[
https://issues.apache.org/jira/browse/HBASE-18095?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033994#comment-17033994
]
Hudson commented on HBASE-18095:
Results for branch HBASE-18095/client-locate-meta-no-zookeeper
[build #69 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/HBASE-18095%252Fclient-locate-meta-no-zookeeper/69/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/HBASE-18095%252Fclient-locate-meta-no-zookeeper/69//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/HBASE-18095%252Fclient-locate-meta-no-zookeeper/69//JDK8_Nightly_Build_Report_(Hadoop2)/]
(x) {color:red}-1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/HBASE-18095%252Fclient-locate-meta-no-zookeeper/69//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(x) {color:red}-1 client integration test{color}
--Failed when running client tests on top of Hadoop 2. [see log for
details|https://builds.apache.org/job/HBase%20Nightly/job/HBASE-18095%252Fclient-locate-meta-no-zookeeper/69//artifact/output-integration/hadoop-2.log].
(note that this means we didn't run on Hadoop 3)
> Provide an option for clients to find the server hosting META that does not
> involve the ZooKeeper client
>
>
> Key: HBASE-18095
> URL: https://issues.apache.org/jira/browse/HBASE-18095
> Project: HBase
> Issue Type: New Feature
> Components: Client
>Reporter: Andrew Kyle Purtell
>Assignee: Bharath Vissapragada
>Priority: Major
> Fix For: 3.0.0, 2.3.0, 1.6.0
>
> Attachments: HBASE-18095.master-v1.patch, HBASE-18095.master-v2.patch
>
>
> Clients are required to connect to ZooKeeper to find the location of the
> regionserver hosting the meta table region. Site configuration provides the
> client a list of ZK quorum peers and the client uses an embedded ZK client to
> query meta location. Timeouts and retry behavior of this embedded ZK client
> are managed orthogonally to HBase layer settings and in some cases the ZK
> cannot manage what in theory the HBase client can, i.e. fail fast upon outage
> or network partition.
> We should consider new configuration settings that provide a list of
> well-known master and backup master locations, and with this information the
> client can contact any of the master processes directly. Any master in either
> active or passive state will track meta location and respond to requests for
> it with its cached last known location. If this location is stale, the client
> can ask again with a flag set that requests the master refresh its location
> cache and return the up-to-date location. Every client interaction with the
> cluster thus uses only HBase RPC as transport, with appropriate settings
> applied to the connection. The configuration toggle that enables this
> alternative meta location lookup should be false by default.
> This removes the requirement that HBase clients embed the ZK client and
> contact the ZK service directly at the beginning of the connection lifecycle.
> This has several benefits. ZK service need not be exposed to clients, and
> their potential abuse, yet no benefit ZK provides the HBase server cluster is
> compromised. Normalizing HBase client and ZK client timeout settings and
> retry behavior - in some cases, impossible, i.e. for fail-fast - is no longer
> necessary.
> And, from [~ghelmling]: There is an additional complication here for
> token-based authentication. When a delegation token is used for SASL
> authentication, the client uses the cluster ID obtained from Zookeeper to
> select the token identifier to use. So there would also need to be some
> Zookeeper-less, unauthenticated way to obtain the cluster ID as well.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (HBASE-23804) Fix default master addr hostname in master registry
[ https://issues.apache.org/jira/browse/HBASE-23804?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Bharath Vissapragada updated HBASE-23804: - Fix Version/s: (was: 2.3.0) (was: 3.0.0) HBASE-18095 Resolution: Fixed Status: Resolved (was: Patch Available) > Fix default master addr hostname in master registry > --- > > Key: HBASE-23804 > URL: https://issues.apache.org/jira/browse/HBASE-23804 > Project: HBase > Issue Type: Bug > Components: Client >Affects Versions: 3.0.0, 2.3.0, 1.6.0, HBASE-18095 >Reporter: Bharath Vissapragada >Assignee: Bharath Vissapragada >Priority: Major > Fix For: HBASE-18095 > > > Currently, master RPC server (*not* info server) always binds to the address > endpoint to which the default hostname of the server resolves to. However, > master registry picks the default end point to connect to as > "localhost:16000" when "hbase.masters" are not configured. This is leading to > a mismatch because the server may not be listening on the loopback address. > This is a problem only in the scripts (single proc/pseudo distributed modes) > because these are the cases in which "hbase.masters" is not populated by > default. > The fix is to pick the service endpoint the same way the RPC server does it. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377364958
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCleanerChore.java
##
@@ -0,0 +1,331 @@
+/**
+ * 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.mob;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.apache.hadoop.hbase.ScheduledChore;
+import org.apache.hadoop.hbase.TableDescriptors;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.backup.HFileArchiver;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.regionserver.BloomType;
+import org.apache.hadoop.hbase.regionserver.HStoreFile;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.hadoop.hbase.util.FSUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+
+/**
+ * The class MobFileCleanerChore for running cleaner regularly to remove the
expired
+ * and obsolete (files which have no active references to) mob files.
+ */
+@InterfaceAudience.Private
+public class MobFileCleanerChore extends ScheduledChore {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MobFileCleanerChore.class);
+ private final HMaster master;
+ private ExpiredMobFileCleaner cleaner;
+
+ static {
+Configuration.addDeprecation(MobConstants.DEPRECATED_MOB_CLEANER_PERIOD,
+ MobConstants.MOB_CLEANER_PERIOD);
+ }
+
+ public MobFileCleanerChore(HMaster master) {
+super(master.getServerName() + "-ExpiredMobFileCleanerChore", master,
+master.getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD,
+ MobConstants.DEFAULT_MOB_CLEANER_PERIOD),
+master.getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD,
+ MobConstants.DEFAULT_MOB_CLEANER_PERIOD),
+TimeUnit.SECONDS);
+this.master = master;
+cleaner = new ExpiredMobFileCleaner();
+cleaner.setConf(master.getConfiguration());
+checkObsoleteConfigurations();
+ }
+
+ private void checkObsoleteConfigurations() {
+Configuration conf = master.getConfiguration();
+
+if (conf.get("hbase.mob.compaction.mergeable.threshold") != null) {
+ LOG.warn("'hbase.mob.compaction.mergeable.threshold' is obsolete and not
used anymore.");
+}
+if (conf.get("hbase.mob.delfile.max.count") != null) {
+ LOG.warn("'hbase.mob.delfile.max.count' is obsolete and not used
anymore.");
+}
+if (conf.get("hbase.mob.compaction.threads.max") != null) {
+ LOG.warn("'hbase.mob.compaction.threads.max' is obsolete and not used
anymore.");
+}
+if (conf.get("hbase.mob.compaction.batch.size") != null) {
+ LOG.warn("'hbase.mob.compaction.batch.size' is obsolete and not used
anymore.");
+}
+ }
+
+ @VisibleForTesting
+ public MobFileCleanerChore() {
+this.master = null;
+ }
+
+ @Override
+ @edu.umd.cs.findbugs.annotations.SuppressWarnings(value =
"REC_CATCH_EXCEPTION",
+ justification = "Intentional")
+
+ protected void chore() {
+TableDescriptors htds = master.getTableDescriptors();
+
+Map map = null;
+try {
+ map = htds.getAll();
+} catch (IOException e) {
+
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377313216
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
##
@@ -183,105 +271,184 @@ protected boolean performCompaction(FileDetails fd,
InternalScanner scanner, Cel
boolean hasMore;
Path path = MobUtils.getMobFamilyPath(conf, store.getTableName(),
store.getColumnFamilyName());
byte[] fileName = null;
-StoreFileWriter mobFileWriter = null, delFileWriter = null;
-long mobCells = 0, deleteMarkersCount = 0;
+StoreFileWriter mobFileWriter = null;
+/*
+ * mobCells are used only to decide if we need to commit or abort current
MOB output file.
+ */
+long mobCells = 0;
long cellsCountCompactedToMob = 0, cellsCountCompactedFromMob = 0;
long cellsSizeCompactedToMob = 0, cellsSizeCompactedFromMob = 0;
boolean finished = false;
+
ScannerContext scannerContext =
ScannerContext.newBuilder().setBatchLimit(compactionKVMax).build();
throughputController.start(compactionName);
-KeyValueScanner kvs = (scanner instanceof KeyValueScanner)?
(KeyValueScanner)scanner : null;
-long shippedCallSizeLimit = (long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+KeyValueScanner kvs = (scanner instanceof KeyValueScanner) ?
(KeyValueScanner) scanner : null;
+long shippedCallSizeLimit =
+(long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+
+Cell mobCell = null;
try {
- try {
-// If the mob file writer could not be created, directly write the
cell to the store file.
-mobFileWriter = mobStore.createWriterInTmp(new Date(fd.latestPutTs),
fd.maxKeyCount,
- compactionCompression, store.getRegionInfo().getStartKey(), true);
-fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
- } catch (IOException e) {
-LOG.warn("Failed to create mob writer, "
- + "we will continue the compaction by writing MOB cells
directly in store files", e);
- }
- if (major) {
-try {
- delFileWriter = mobStore.createDelFileWriterInTmp(new
Date(fd.latestPutTs),
-fd.maxKeyCount, compactionCompression,
store.getRegionInfo().getStartKey());
-} catch (IOException e) {
- LOG.warn(
-"Failed to create del writer, "
-+ "we will continue the compaction by writing delete markers
directly in store files",
-e);
-}
- }
+
+ mobFileWriter = newMobWriter(fd);
+ fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
+
do {
hasMore = scanner.next(cells, scannerContext);
-if (LOG.isDebugEnabled()) {
- now = EnvironmentEdgeManager.currentTime();
-}
+now = EnvironmentEdgeManager.currentTime();
for (Cell c : cells) {
- if (major && CellUtil.isDelete(c)) {
-if (MobUtils.isMobReferenceCell(c) || delFileWriter == null) {
- // Directly write it to a store file
- writer.append(c);
+ if (compactMOBs) {
+if (MobUtils.isMobReferenceCell(c)) {
+ String fName = MobUtils.getMobFileName(c);
+ Path pp = new Path(new Path(fs.getUri()), new Path(path, fName));
+
+ // Added to support migration
+ try {
+mobCell = mobStore.resolve(c, true, false).getCell();
+ } catch (FileNotFoundException fnfe) {
+if (discardMobMiss) {
+ LOG.error("Missing MOB cell: file={} not found cell={}",
fName, c);
+ continue;
+} else {
+ throw fnfe;
+}
+ }
+
+ if (discardMobMiss && mobCell.getValueLength() == 0) {
+LOG.error("Missing MOB cell value: file={} cell={}", pp,
mobCell);
Review comment:
mobCell is Cell object
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377317451
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
##
@@ -183,105 +271,184 @@ protected boolean performCompaction(FileDetails fd,
InternalScanner scanner, Cel
boolean hasMore;
Path path = MobUtils.getMobFamilyPath(conf, store.getTableName(),
store.getColumnFamilyName());
byte[] fileName = null;
-StoreFileWriter mobFileWriter = null, delFileWriter = null;
-long mobCells = 0, deleteMarkersCount = 0;
+StoreFileWriter mobFileWriter = null;
+/*
+ * mobCells are used only to decide if we need to commit or abort current
MOB output file.
+ */
+long mobCells = 0;
long cellsCountCompactedToMob = 0, cellsCountCompactedFromMob = 0;
long cellsSizeCompactedToMob = 0, cellsSizeCompactedFromMob = 0;
boolean finished = false;
+
ScannerContext scannerContext =
ScannerContext.newBuilder().setBatchLimit(compactionKVMax).build();
throughputController.start(compactionName);
-KeyValueScanner kvs = (scanner instanceof KeyValueScanner)?
(KeyValueScanner)scanner : null;
-long shippedCallSizeLimit = (long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+KeyValueScanner kvs = (scanner instanceof KeyValueScanner) ?
(KeyValueScanner) scanner : null;
+long shippedCallSizeLimit =
+(long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+
+Cell mobCell = null;
try {
- try {
-// If the mob file writer could not be created, directly write the
cell to the store file.
-mobFileWriter = mobStore.createWriterInTmp(new Date(fd.latestPutTs),
fd.maxKeyCount,
- compactionCompression, store.getRegionInfo().getStartKey(), true);
-fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
- } catch (IOException e) {
-LOG.warn("Failed to create mob writer, "
- + "we will continue the compaction by writing MOB cells
directly in store files", e);
- }
- if (major) {
-try {
- delFileWriter = mobStore.createDelFileWriterInTmp(new
Date(fd.latestPutTs),
-fd.maxKeyCount, compactionCompression,
store.getRegionInfo().getStartKey());
-} catch (IOException e) {
- LOG.warn(
-"Failed to create del writer, "
-+ "we will continue the compaction by writing delete markers
directly in store files",
-e);
-}
- }
+
+ mobFileWriter = newMobWriter(fd);
+ fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
+
do {
hasMore = scanner.next(cells, scannerContext);
-if (LOG.isDebugEnabled()) {
- now = EnvironmentEdgeManager.currentTime();
-}
+now = EnvironmentEdgeManager.currentTime();
for (Cell c : cells) {
- if (major && CellUtil.isDelete(c)) {
-if (MobUtils.isMobReferenceCell(c) || delFileWriter == null) {
- // Directly write it to a store file
- writer.append(c);
+ if (compactMOBs) {
+if (MobUtils.isMobReferenceCell(c)) {
+ String fName = MobUtils.getMobFileName(c);
+ Path pp = new Path(new Path(fs.getUri()), new Path(path, fName));
+
+ // Added to support migration
+ try {
+mobCell = mobStore.resolve(c, true, false).getCell();
+ } catch (FileNotFoundException fnfe) {
+if (discardMobMiss) {
+ LOG.error("Missing MOB cell: file={} not found cell={}",
fName, c);
+ continue;
+} else {
+ throw fnfe;
+}
+ }
+
+ if (discardMobMiss && mobCell.getValueLength() == 0) {
+LOG.error("Missing MOB cell value: file={} cell={}", pp,
mobCell);
+continue;
+ } else if (mobCell.getValueLength() == 0) {
+String errMsg = String.format("Found 0 length MOB cell in a
file=%s cell=%s",
+ fName, mobCell);
+throw new IOException(errMsg);
+ }
+
+ if (mobCell.getValueLength() > mobSizeThreshold) {
+// put the mob data back to the MOB store file
+PrivateCellUtil.setSequenceId(mobCell, c.getSequenceId());
+if (!ioOptimizedMode) {
+ mobFileWriter.append(mobCell);
+ mobCells++;
+ writer.append(
+MobUtils.createMobRefCell(mobCell, fileName,
this.mobStore.getRefCellTags()));
+} else {
+ // I/O optimized mode
+ // Check if MOB cell origin file size is
+ // greater than threshold
+
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377317317
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
##
@@ -183,105 +271,184 @@ protected boolean performCompaction(FileDetails fd,
InternalScanner scanner, Cel
boolean hasMore;
Path path = MobUtils.getMobFamilyPath(conf, store.getTableName(),
store.getColumnFamilyName());
byte[] fileName = null;
-StoreFileWriter mobFileWriter = null, delFileWriter = null;
-long mobCells = 0, deleteMarkersCount = 0;
+StoreFileWriter mobFileWriter = null;
+/*
+ * mobCells are used only to decide if we need to commit or abort current
MOB output file.
+ */
+long mobCells = 0;
long cellsCountCompactedToMob = 0, cellsCountCompactedFromMob = 0;
long cellsSizeCompactedToMob = 0, cellsSizeCompactedFromMob = 0;
boolean finished = false;
+
ScannerContext scannerContext =
ScannerContext.newBuilder().setBatchLimit(compactionKVMax).build();
throughputController.start(compactionName);
-KeyValueScanner kvs = (scanner instanceof KeyValueScanner)?
(KeyValueScanner)scanner : null;
-long shippedCallSizeLimit = (long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+KeyValueScanner kvs = (scanner instanceof KeyValueScanner) ?
(KeyValueScanner) scanner : null;
+long shippedCallSizeLimit =
+(long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+
+Cell mobCell = null;
try {
- try {
-// If the mob file writer could not be created, directly write the
cell to the store file.
-mobFileWriter = mobStore.createWriterInTmp(new Date(fd.latestPutTs),
fd.maxKeyCount,
- compactionCompression, store.getRegionInfo().getStartKey(), true);
-fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
- } catch (IOException e) {
-LOG.warn("Failed to create mob writer, "
- + "we will continue the compaction by writing MOB cells
directly in store files", e);
- }
- if (major) {
-try {
- delFileWriter = mobStore.createDelFileWriterInTmp(new
Date(fd.latestPutTs),
-fd.maxKeyCount, compactionCompression,
store.getRegionInfo().getStartKey());
-} catch (IOException e) {
- LOG.warn(
-"Failed to create del writer, "
-+ "we will continue the compaction by writing delete markers
directly in store files",
-e);
-}
- }
+
+ mobFileWriter = newMobWriter(fd);
+ fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
+
do {
hasMore = scanner.next(cells, scannerContext);
-if (LOG.isDebugEnabled()) {
- now = EnvironmentEdgeManager.currentTime();
-}
+now = EnvironmentEdgeManager.currentTime();
for (Cell c : cells) {
- if (major && CellUtil.isDelete(c)) {
-if (MobUtils.isMobReferenceCell(c) || delFileWriter == null) {
- // Directly write it to a store file
- writer.append(c);
+ if (compactMOBs) {
+if (MobUtils.isMobReferenceCell(c)) {
+ String fName = MobUtils.getMobFileName(c);
+ Path pp = new Path(new Path(fs.getUri()), new Path(path, fName));
+
+ // Added to support migration
+ try {
+mobCell = mobStore.resolve(c, true, false).getCell();
+ } catch (FileNotFoundException fnfe) {
+if (discardMobMiss) {
+ LOG.error("Missing MOB cell: file={} not found cell={}",
fName, c);
+ continue;
+} else {
+ throw fnfe;
+}
+ }
+
+ if (discardMobMiss && mobCell.getValueLength() == 0) {
+LOG.error("Missing MOB cell value: file={} cell={}", pp,
mobCell);
+continue;
+ } else if (mobCell.getValueLength() == 0) {
+String errMsg = String.format("Found 0 length MOB cell in a
file=%s cell=%s",
+ fName, mobCell);
+throw new IOException(errMsg);
+ }
+
+ if (mobCell.getValueLength() > mobSizeThreshold) {
+// put the mob data back to the MOB store file
+PrivateCellUtil.setSequenceId(mobCell, c.getSequenceId());
+if (!ioOptimizedMode) {
+ mobFileWriter.append(mobCell);
+ mobCells++;
+ writer.append(
+MobUtils.createMobRefCell(mobCell, fileName,
this.mobStore.getRefCellTags()));
+} else {
+ // I/O optimized mode
+ // Check if MOB cell origin file size is
+ // greater than threshold
+
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377238105
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
##
@@ -496,11 +518,51 @@ void logResponse(Message param, String methodName,
String call, String tag,
}
}
}
- responseInfo.put("multi.gets", numGets);
- responseInfo.put("multi.mutations", numMutations);
- responseInfo.put("multi.servicecalls", numServiceCalls);
+ responseInfo.put(MULTI_GETS, numGets);
+ responseInfo.put(MULTI_MUTATIONS, numMutations);
+ responseInfo.put(MULTI_SERVICE_CALLS, numServiceCalls);
}
+final String tag = tooLarge ? "TooLarge" : "TooSlow";
LOG.warn("(response" + tag + "): " + GSON.toJson(responseInfo));
+if (tooSlow && this.onlineSlowLogProvider != null
+&& !call.startsWith(GET_SLOW_LOG_RESPONSES)
+&& !call.startsWith(CLEAR_SLOW_LOGS_RESPONSES)) {
+ logOnlineSlowResponse(param, methodName, call, clientAddress, startTime,
+processingTime, qTime, responseSize, userName, className,
responseInfo);
+}
+ }
+
+ private void logOnlineSlowResponse(Message param, String methodName, String
call,
+ String clientAddress, long startTime, int processingTime, int qTime,
long responseSize,
+ String userName, String className, Map responseInfo) {
+// add too slow log to ringbuffer for retrieval of latest n slow logs
+
+try {
+ final SlowLogParams slowLogParams = ProtobufUtil.getSlowLogParams(param);
+
+ final SlowLogPayload slowLogPayload = SlowLogPayload.newBuilder()
+.setStartTime(startTime)
+.setProcessingTime(processingTime)
+.setQueueTime(qTime)
+.setResponseSize(responseSize)
+.setClientAddress(clientAddress)
+.setServerClass(className)
+.setMethodName(methodName)
+.setCallDetails(call)
+.setUserName(userName)
+.setRegionName(slowLogParams != null ? slowLogParams.getRegionName() :
StringUtils.EMPTY)
+.setParam(slowLogParams != null ? slowLogParams.getParams() :
StringUtils.EMPTY)
+.setMultiGets(responseInfo.containsKey(MULTI_GETS)
+ ? (int) responseInfo.get(MULTI_GETS) : 0)
+.setMultiMutations(responseInfo.containsKey(MULTI_MUTATIONS)
+ ? (int) responseInfo.get(MULTI_MUTATIONS) : 0)
+.setMultiServiceCalls(responseInfo.containsKey(MULTI_SERVICE_CALLS)
+ ? (int) responseInfo.get(MULTI_SERVICE_CALLS) : 0)
+.build();
+ this.onlineSlowLogProvider.addSlowLogPayload(slowLogPayload);
+} catch (Exception e) {
+ LOG.debug("Error while adding slowlog response to ringbuffer", e);
Review comment:
If an operator has enabled this feature, I think they'd want to know if it's
somehow not working. How about logging at warning level instead?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377221782
##
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/SlowLogPayload.java
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * 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.client;
+
+import org.apache.hadoop.hbase.util.GsonUtil;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hbase.thirdparty.com.google.gson.Gson;
+import org.apache.hbase.thirdparty.com.google.gson.JsonObject;
+import org.apache.hbase.thirdparty.com.google.gson.JsonSerializer;
+
+/**
+ * SlowLog payload for client
+ */
+@InterfaceAudience.Private
+final public class SlowLogPayload {
+
+ private static final Gson GSON = GsonUtil.createGson()
+.setPrettyPrinting()
+.registerTypeAdapter(SlowLogPayload.class, (JsonSerializer)
+ (slowLogPayload, type, jsonSerializationContext) -> {
Review comment:
I don't know Gson API; I'm more familiar with Jackson, which has lots of
little annotations for easily tweaking the details of serialization.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377247463
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/RingBufferTruck.java
##
@@ -0,0 +1,63 @@
+/*
+ *
+ * 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.slowlog;
+
+import org.apache.yetus.audience.InterfaceAudience;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * A truck to carry payload in the slow log ring buffer that serves as online
buffer
+ * to provide latest TooSlowLog
+ */
+@InterfaceAudience.Private
+final class RingBufferTruck {
+
+ private SlowLogPayload slowLogPayload;
+
+ /**
+ * Load the Truck with {@link SlowLogPayload}
+ *
+ * @param slowLogPayload Payload to load Truck with
+ */
+ public void load(SlowLogPayload slowLogPayload) {
+this.slowLogPayload = slowLogPayload;
+ }
+
+ /**
+ * Retrieve current payload {@link SlowLogPayload} available on Truck and
+ * free up the Truck
+ *
+ * @return Retrieve available payload
+ */
+ public SlowLogPayload getPayload() {
+final SlowLogPayload slowLogPayload = this.slowLogPayload;
+this.slowLogPayload = null;
+return slowLogPayload;
+ }
+
+ /**
+ * To clean up the payload, clear payload reference
+ */
+ public void clearPayload() {
Review comment:
unused.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377225705
##
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/SlowLogPayload.java
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * 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.client;
+
+import org.apache.hadoop.hbase.util.GsonUtil;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hbase.thirdparty.com.google.gson.Gson;
+import org.apache.hbase.thirdparty.com.google.gson.JsonObject;
+import org.apache.hbase.thirdparty.com.google.gson.JsonSerializer;
+
+/**
+ * SlowLog payload for client
+ */
+@InterfaceAudience.Private
+final public class SlowLogPayload {
+
+ private static final Gson GSON = GsonUtil.createGson()
+.setPrettyPrinting()
+.registerTypeAdapter(SlowLogPayload.class, (JsonSerializer)
+ (slowLogPayload, type, jsonSerializationContext) -> {
+Gson gson = new Gson();
+JsonObject jsonObj = (JsonObject) gson.toJsonTree(slowLogPayload);
+if (slowLogPayload.getMultiGetsCount() == 0) {
+ jsonObj.remove("multiGetsCount");
+}
+if (slowLogPayload.getMultiMutationsCount() == 0) {
+ jsonObj.remove("multiMutationsCount");
+}
+if (slowLogPayload.getMultiServiceCalls() == 0) {
+ jsonObj.remove("multiServiceCalls");
+}
+return jsonObj;
+ }).create();
+
+ private long startTime;
+ private int processingTime;
+ private int queueTime;
+ private long responseSize;
+ private String clientAddress;
+ private String serverClass;
+ private String methodName;
+ private String callDetails;
+ private String param;
+ private transient String regionName;
+ private String userName;
+ private int multiGetsCount;
+ private int multiMutationsCount;
+ private int multiServiceCalls;
+
+ public long getStartTime() {
+return startTime;
+ }
+
+ public int getProcessingTime() {
+return processingTime;
+ }
+
+ public int getQueueTime() {
+return queueTime;
+ }
+
+ public long getResponseSize() {
+return responseSize;
+ }
+
+ public String getClientAddress() {
+return clientAddress;
+ }
+
+ public String getServerClass() {
+return serverClass;
+ }
+
+ public String getMethodName() {
+return methodName;
+ }
+
+ public String getCallDetails() {
+return callDetails;
+ }
+
+ public String getParam() {
+return param;
+ }
+
+ public String getRegionName() {
+return regionName;
+ }
+
+ public String getUserName() {
+return userName;
+ }
+
+ public int getMultiGetsCount() {
+return multiGetsCount;
+ }
+
+ public int getMultiMutationsCount() {
+return multiMutationsCount;
+ }
+
+ public int getMultiServiceCalls() {
+return multiServiceCalls;
+ }
+
+ private SlowLogPayload(final long startTime, final int processingTime, final
int queueTime,
+ final long responseSize, final String clientAddress, final String
serverClass,
+ final String methodName, final String callDetails, final String param,
+ final String regionName, final String userName, final int multiGetsCount,
+ final int multiMutationsCount, final int multiServiceCalls) {
+this.startTime = startTime;
+this.processingTime = processingTime;
+this.queueTime = queueTime;
+this.responseSize = responseSize;
+this.clientAddress = clientAddress;
+this.serverClass = serverClass;
+this.methodName = methodName;
+this.callDetails = callDetails;
+this.param = param;
+this.regionName = regionName;
+this.userName = userName;
+this.multiGetsCount = multiGetsCount;
+this.multiMutationsCount = multiMutationsCount;
+this.multiServiceCalls = multiServiceCalls;
+ }
+
+ public static class SlowLogPayloadBuilder {
+private long startTime;
+private int processingTime;
+private int queueTime;
+private long responseSize;
+private String clientAddress;
+private String serverClass;
+private String methodName;
+private String callDetails;
+private String param;
+private String regionName;
+
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377220054
##
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
##
@@ -3889,4 +3891,69 @@ private void getProcedureResult(long procId,
CompletableFuture future, int
.call();
}
+ @Override
+ public CompletableFuture> getSlowLogResponses(
+ List serverNames) {
+CompletableFuture> slowLoadPayloads =
+ CompletableFuture.completedFuture(Collections.emptyList());
+if (CollectionUtils.isEmpty(serverNames)) {
+ return slowLoadPayloads;
+}
+for (ServerName serverName : serverNames) {
+ CompletableFuture> slowLogPayloadFromServer =
+getSlowLogResponseFromServer(serverName);
+ slowLoadPayloads =
slowLoadPayloads.thenCombine(slowLogPayloadFromServer, (l1, l2) -> {
+List finalSlowLogPayloads = new LinkedList<>();
+finalSlowLogPayloads.addAll(l1);
+finalSlowLogPayloads.addAll(l2);
+return finalSlowLogPayloads;
+ });
+}
+return slowLoadPayloads;
+ }
+
+ private CompletableFuture> getSlowLogResponseFromServer(
+ final ServerName serverName) {
+return this.>newAdminCaller()
+ .action((controller, stub) -> this
+.adminCall(
+ controller, stub, RequestConverter.buildSlowLogResponseRequest(),
+ AdminService.Interface::getSlowLogResponses,
+ ProtobufUtil::toSlowLogPayloads))
+ .serverName(serverName).call();
+ }
+
+ @Override
+ public CompletableFuture>
clearSlowLogResponses(List serverNames) {
+List> clearSlowLogResponseList = new
ArrayList<>();
+if (CollectionUtils.isNotEmpty(serverNames)) {
+ for (ServerName serverName : serverNames) {
Review comment:
nit: you can probably collapse this for-loop and `convertToFutureOfList`
into a single `stream()` incantation that will be easier to read.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377236616
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
##
@@ -496,11 +518,51 @@ void logResponse(Message param, String methodName,
String call, String tag,
}
}
}
- responseInfo.put("multi.gets", numGets);
- responseInfo.put("multi.mutations", numMutations);
- responseInfo.put("multi.servicecalls", numServiceCalls);
+ responseInfo.put(MULTI_GETS, numGets);
+ responseInfo.put(MULTI_MUTATIONS, numMutations);
+ responseInfo.put(MULTI_SERVICE_CALLS, numServiceCalls);
}
+final String tag = tooLarge ? "TooLarge" : "TooSlow";
LOG.warn("(response" + tag + "): " + GSON.toJson(responseInfo));
+if (tooSlow && this.onlineSlowLogProvider != null
+&& !call.startsWith(GET_SLOW_LOG_RESPONSES)
+&& !call.startsWith(CLEAR_SLOW_LOGS_RESPONSES)) {
+ logOnlineSlowResponse(param, methodName, call, clientAddress, startTime,
+processingTime, qTime, responseSize, userName, className,
responseInfo);
+}
+ }
+
+ private void logOnlineSlowResponse(Message param, String methodName, String
call,
+ String clientAddress, long startTime, int processingTime, int qTime,
long responseSize,
+ String userName, String className, Map responseInfo) {
+// add too slow log to ringbuffer for retrieval of latest n slow logs
Review comment:
Might as well make this a javadoc comment on the method.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377226196
##
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/SlowLogPayload.java
##
@@ -0,0 +1,318 @@
+/**
+ *
+ * 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.client;
+
+import org.apache.hadoop.hbase.util.GsonUtil;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import org.apache.hbase.thirdparty.com.google.gson.Gson;
+import org.apache.hbase.thirdparty.com.google.gson.JsonObject;
+import org.apache.hbase.thirdparty.com.google.gson.JsonSerializer;
+
+/**
+ * SlowLog payload for client
+ */
+@InterfaceAudience.Private
+final public class SlowLogPayload {
+
+ private static final Gson GSON = GsonUtil.createGson()
+.setPrettyPrinting()
+.registerTypeAdapter(SlowLogPayload.class, (JsonSerializer)
+ (slowLogPayload, type, jsonSerializationContext) -> {
+Gson gson = new Gson();
+JsonObject jsonObj = (JsonObject) gson.toJsonTree(slowLogPayload);
+if (slowLogPayload.getMultiGetsCount() == 0) {
+ jsonObj.remove("multiGetsCount");
+}
+if (slowLogPayload.getMultiMutationsCount() == 0) {
+ jsonObj.remove("multiMutationsCount");
+}
+if (slowLogPayload.getMultiServiceCalls() == 0) {
+ jsonObj.remove("multiServiceCalls");
+}
+return jsonObj;
+ }).create();
+
+ private long startTime;
+ private int processingTime;
+ private int queueTime;
+ private long responseSize;
+ private String clientAddress;
+ private String serverClass;
+ private String methodName;
+ private String callDetails;
+ private String param;
+ private transient String regionName;
+ private String userName;
+ private int multiGetsCount;
+ private int multiMutationsCount;
+ private int multiServiceCalls;
+
+ public long getStartTime() {
+return startTime;
+ }
+
+ public int getProcessingTime() {
+return processingTime;
+ }
+
+ public int getQueueTime() {
+return queueTime;
+ }
+
+ public long getResponseSize() {
+return responseSize;
+ }
+
+ public String getClientAddress() {
+return clientAddress;
+ }
+
+ public String getServerClass() {
+return serverClass;
+ }
+
+ public String getMethodName() {
+return methodName;
+ }
+
+ public String getCallDetails() {
+return callDetails;
+ }
+
+ public String getParam() {
+return param;
+ }
+
+ public String getRegionName() {
+return regionName;
+ }
+
+ public String getUserName() {
+return userName;
+ }
+
+ public int getMultiGetsCount() {
+return multiGetsCount;
+ }
+
+ public int getMultiMutationsCount() {
+return multiMutationsCount;
+ }
+
+ public int getMultiServiceCalls() {
+return multiServiceCalls;
+ }
+
+ private SlowLogPayload(final long startTime, final int processingTime, final
int queueTime,
+ final long responseSize, final String clientAddress, final String
serverClass,
+ final String methodName, final String callDetails, final String param,
+ final String regionName, final String userName, final int multiGetsCount,
+ final int multiMutationsCount, final int multiServiceCalls) {
+this.startTime = startTime;
+this.processingTime = processingTime;
+this.queueTime = queueTime;
+this.responseSize = responseSize;
+this.clientAddress = clientAddress;
+this.serverClass = serverClass;
+this.methodName = methodName;
+this.callDetails = callDetails;
+this.param = param;
+this.regionName = regionName;
+this.userName = userName;
+this.multiGetsCount = multiGetsCount;
+this.multiMutationsCount = multiMutationsCount;
+this.multiServiceCalls = multiServiceCalls;
+ }
+
+ public static class SlowLogPayloadBuilder {
+private long startTime;
+private int processingTime;
+private int queueTime;
+private long responseSize;
+private String clientAddress;
+private String serverClass;
+private String methodName;
+private String callDetails;
+private String param;
+private String regionName;
+
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377235972
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
##
@@ -496,11 +518,51 @@ void logResponse(Message param, String methodName,
String call, String tag,
}
}
}
- responseInfo.put("multi.gets", numGets);
- responseInfo.put("multi.mutations", numMutations);
- responseInfo.put("multi.servicecalls", numServiceCalls);
+ responseInfo.put(MULTI_GETS, numGets);
+ responseInfo.put(MULTI_MUTATIONS, numMutations);
+ responseInfo.put(MULTI_SERVICE_CALLS, numServiceCalls);
}
+final String tag = tooLarge ? "TooLarge" : "TooSlow";
LOG.warn("(response" + tag + "): " + GSON.toJson(responseInfo));
+if (tooSlow && this.onlineSlowLogProvider != null
+&& !call.startsWith(GET_SLOW_LOG_RESPONSES)
Review comment:
Why exclude these? It's quite possible that these RPCs could be taking a
long 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377245309
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/OnlineSlowLogProvider.java
##
@@ -0,0 +1,137 @@
+/**
+ *
+ * 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.slowlog;
+
+import com.lmax.disruptor.BlockingWaitStrategy;
+import com.lmax.disruptor.RingBuffer;
+import com.lmax.disruptor.dsl.Disruptor;
+import com.lmax.disruptor.dsl.ProducerType;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.util.Threads;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.yetus.audience.InterfaceStability;
+
+import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Online SlowLog Provider Service
+ */
+@InterfaceAudience.Private
+@InterfaceStability.Evolving
+public class OnlineSlowLogProvider {
+
+ private final Disruptor disruptor;
+ private final SlowLogEventHandler slowLogEventHandler;
+ private final int eventCount;
+ private final boolean isOnlineSlowLogProviderEnabled;
+
+ /**
+ * Initialize disruptor with configurable ringbuffer size
+ */
+ public OnlineSlowLogProvider(Configuration conf) {
+isOnlineSlowLogProviderEnabled =
conf.getBoolean(HConstants.SLOW_LOG_BUFFER_ENABLED_KEY,
+ HConstants.DEFAULT_ONLINE_LOG_PROVIDER_ENABLED);
+
+this.eventCount = conf.getInt(HConstants.SLOW_LOG_RING_BUFFER_SIZE,
+ HConstants.DEFAULT_SLOW_LOG_RING_BUFFER_SIZE);
+
+// This is the 'writer' -- a single threaded executor. This single thread
consumes what is
+// put on the ringbuffer.
+final String hostingThreadName = Thread.currentThread().getName();
+
+// disruptor initialization with BlockingWaitStrategy
+this.disruptor = new Disruptor<>(RingBufferTruck::new,
+ getEventCount(),
+ Threads.newDaemonThreadFactory(hostingThreadName + ".append"),
Review comment:
Maybe include something to the effect of ".slowlog.append" so that it's not
confused as a data-path append thread.
I'm curious what the parent thread name ends up being...
`"RS:0;localhost:"`?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377252533
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogEventHandler.java
##
@@ -0,0 +1,137 @@
+/*
+ *
+ * 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.slowlog;
+
+import com.lmax.disruptor.EventHandler;
+import com.lmax.disruptor.RingBuffer;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.stream.Collectors;
+
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Event Handler run by disruptor ringbuffer consumer
+ */
+@InterfaceAudience.Private
+class SlowLogEventHandler implements EventHandler {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(SlowLogEventHandler.class);
+
+ private final SlowLogPayload[] slowLogPayloads;
+
+ private AtomicInteger slowLogPayloadIdx = new AtomicInteger();
+
+ private static final Lock LOCK = new ReentrantLock();
+
+ private static final int WRITE_LOCK_WAIT = 1;
Review comment:
These are quantities of a unit of time; include that unit in the name.
`WRITE_LOCK_WAIT_SEC`
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377213128
##
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
##
@@ -3889,4 +3891,69 @@ private void getProcedureResult(long procId,
CompletableFuture future, int
.call();
}
+ @Override
+ public CompletableFuture> getSlowLogResponses(
+ List serverNames) {
+CompletableFuture> slowLoadPayloads =
+ CompletableFuture.completedFuture(Collections.emptyList());
+if (CollectionUtils.isEmpty(serverNames)) {
+ return slowLoadPayloads;
+}
+for (ServerName serverName : serverNames) {
+ CompletableFuture> slowLogPayloadFromServer =
+getSlowLogResponseFromServer(serverName);
+ slowLoadPayloads =
slowLoadPayloads.thenCombine(slowLogPayloadFromServer, (l1, l2) -> {
+List finalSlowLogPayloads = new LinkedList<>();
Review comment:
nit: For larger clusters, you can save yourself a lot of extra allocations
by using a stream-based approach, something like
```java
if (CollectionUtils.isEmpty(serverNames)) {
return CompletableFuture.completedFuture(Collections.emptyList());
}
return CompletableFuture.supplyAsync(() -> {
return serverNames.stream()
.map(this::getSlowLogResponseFromServer)
.map(CompletableFuture::join)
.flatMap(List::stream)
.collect(Collectors.toList());
});
```
Actually, what I think you really want is a solution that won't fall over in
the face of a very large cluster with lots of slow results. Such a solution
would involve a user-provided limit on the number of results returned,
partitioning the server list in batches of size N, processing a batch, and
short-circuiting the return when a result-count limit is reached. So, to
support this use case, you'll need to add a limit parameter to the methods (and
maybe a configuration point providing a default limit) and then implement
pagination (which I think can also be implemented as a stream over partitions
plus `Stream.limit`).
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377233563
##
File path: hbase-protocol-shaded/src/main/protobuf/TooSlowLog.proto
##
@@ -0,0 +1,45 @@
+/**
+ * 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.
+ */
+
+syntax = "proto2";
+
+// This file contains protocol buffers that are used for Online TooSlowLogs
+// To be used as Ring Buffer payload
+package hbase.pb;
+
+option java_package = "org.apache.hadoop.hbase.shaded.protobuf.generated";
+option java_outer_classname = "TooSlowLog";
+option java_generate_equals_and_hash = true;
+option optimize_for = SPEED;
+
+message SlowLogPayload {
+required int64 start_time = 1;
+required int32 processing_time = 2;
+required int32 queue_time = 3;
+required int64 response_size = 4;
+required string client_address = 5;
+required string server_class = 6;
+required string method_name = 7;
+required string call_details = 8;
+required string param = 9;
+required string user_name = 10;
Review comment:
So this POJO is used both for storing/serializing values and also as the
query rpc filter params list? Maybe those two concerns should have two
different payload objects...
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log URL: https://github.com/apache/hbase/pull/754#discussion_r377231221 ## File path: hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java ## @@ -1519,6 +1519,15 @@ "hbase.master.executor.logreplayops.threads"; public static final int MASTER_LOG_REPLAY_OPS_THREADS_DEFAULT = 10; + public static final String SLOW_LOG_RING_BUFFER_SIZE = Review comment: These constants are consumed in just one place, so they should be moved to `OnlineSlowLogProvider`, the class that they influence. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log URL: https://github.com/apache/hbase/pull/754#discussion_r377240740 ## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java ## @@ -215,6 +224,17 @@ */ private RSRpcServices rsRpcServices; + + /** + * Use to add online slowlog responses + */ + private OnlineSlowLogProvider onlineSlowLogProvider; Review comment: Oh I see... yeah this is rough. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log URL: https://github.com/apache/hbase/pull/754#discussion_r377272041 ## File path: hbase-shell/src/main/ruby/shell/commands/get_slowlog_responses.rb ## @@ -0,0 +1,65 @@ +# +# +# 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. + +# Retrieve latest slowlog responses maintained in memory by RegionServers + +module Shell + module Commands +# Retrieve latest slowlog responses +class GetSlowlogResponses < Command + def help +<<-EOF +Retrieve latest SlowLog Responses maintained by each or specific RegionServers. +Specify '*' to include all RS otherwise array of server names for specific +RS. A server name is the host, port plus startcode of a RegionServer. +e.g.: host187.example.com,60020,1289493121758 (find servername in +master ui or when you do detailed status in shell) + +Provide optional filter parameters as Hash + +Examples: Review comment: Nice docs. Examples really help. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log URL: https://github.com/apache/hbase/pull/754#discussion_r377271382 ## File path: hbase-shell/src/main/ruby/hbase/admin.rb ## @@ -1527,6 +1618,16 @@ def stop_master def stop_regionserver(hostport) @admin.stopRegionServer(hostport) end + + #-- +# Get list of server names +def get_server_names(server_names) Review comment: nit: this is more of a `_to_java` kind of method. Reconsider the name. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377254854
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogEventHandler.java
##
@@ -0,0 +1,137 @@
+/*
+ *
+ * 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.slowlog;
+
+import com.lmax.disruptor.EventHandler;
+import com.lmax.disruptor.RingBuffer;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.stream.Collectors;
+
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Event Handler run by disruptor ringbuffer consumer
+ */
+@InterfaceAudience.Private
+class SlowLogEventHandler implements EventHandler {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(SlowLogEventHandler.class);
+
+ private final SlowLogPayload[] slowLogPayloads;
+
+ private AtomicInteger slowLogPayloadIdx = new AtomicInteger();
+
+ private static final Lock LOCK = new ReentrantLock();
+
+ private static final int WRITE_LOCK_WAIT = 1;
+ private static final int READ_LOCK_WAIT = 1;
+
+ SlowLogEventHandler(int eventCount) {
+this.slowLogPayloads = new SlowLogPayload[eventCount];
+ }
+
+ /**
+ * Called when a publisher has published an event to the {@link RingBuffer}
+ *
+ * @param event published to the {@link RingBuffer}
+ * @param sequence of the event being processed
+ * @param endOfBatch flag to indicate if this is the last event in a batch
from
+ * the {@link RingBuffer}
+ * @throws Exception if the EventHandler would like the exception handled
further up the chain
+ */
+ @Override
+ public void onEvent(RingBufferTruck event, long sequence, boolean
endOfBatch) throws Exception {
+SlowLogPayload slowLogPayload = event.getPayload();
+if (LOG.isTraceEnabled()) {
+ LOG.trace("Received Slow Log Event. RingBuffer sequence: {},
isEndOfBatch: {}, " +
+ "Event Call: {}", sequence, endOfBatch,
+slowLogPayload.getCallDetails());
+}
+try {
+ if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
+int index = slowLogPayloadIdx.getAndSet(
+ (slowLogPayloadIdx.get() + 1) % slowLogPayloads.length);
+slowLogPayloads[index] = slowLogPayload;
+ }
+} finally {
+ LOCK.unlock();
+}
+ }
+
+ /**
+ * Cleans up slow log payloads
+ *
+ * @return true if slow log payloads are cleaned up, false otherwise
+ */
+ boolean clearSlowLogs() {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("Received request to clean up online slowlog buffer..");
+}
+boolean isCleanedUp = true;
+try {
+ if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
+for (int i = 0; i < slowLogPayloads.length; i++) {
+ slowLogPayloads[i] = null;
+}
+slowLogPayloadIdx.set(0);
+ }
Review comment:
same lack of `else` clause.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377268619
##
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/slowlog/TestOnlineSlowLogProvider.java
##
@@ -0,0 +1,307 @@
+/*
+ *
+ * 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.slowlog;
+
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.Assert;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.util.concurrent.Uninterruptibles;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Tests for Online SlowLog Provider Service
+ */
+@Category({MasterTests.class, MediumTests.class})
+public class TestOnlineSlowLogProvider {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+HBaseClassTestRule.forClass(TestOnlineSlowLogProvider.class);
+
+ private static final Logger LOG =
LoggerFactory.getLogger(TestOnlineSlowLogProvider.class);
+
+ private static final HBaseTestingUtility HBASE_TESTING_UTILITY = new
HBaseTestingUtility();
+
+ private OnlineSlowLogProvider onlineSlowLogProvider;
+
+ private Configuration getOnlineSlowLogConf(int eventSize) {
+Configuration conf = HBASE_TESTING_UTILITY.getConfiguration();
+conf.setBoolean(HConstants.SLOW_LOG_BUFFER_ENABLED_KEY, true);
+conf.setInt(HConstants.SLOW_LOG_RING_BUFFER_SIZE, eventSize);
+return conf;
+ }
+
+ private SlowLogPayload getSlowLogPayload(int i) {
+SlowLogPayload slowLogPayload = SlowLogPayload.newBuilder()
+ .setCallDetails("call_" + i)
+ .setClientAddress("client_" + i)
+ .setMethodName("method_" + i)
+ .setUserName("userName_" + i)
+ .setMultiGets(i)
+ .setMultiMutations(i)
+ .setMultiServiceCalls(i)
+ .setProcessingTime(i + 500)
+ .setQueueTime(i + 400)
+ .setResponseSize(i + 200)
+ .setStartTime(EnvironmentEdgeManager.currentTime())
+ .setServerClass("class_" + i)
+ .setParam("param_" + i)
+ .build();
+return slowLogPayload;
+ }
+
+ /**
+ * confirm that for a ringbuffer of slow logs, payload on given index of
buffer
+ * has expected elements
+ *
+ * @param i index of ringbuffer logs
+ * @param j data value that was put on index i
+ * @param slowLogPayloads list of payload retrieved from {@link
OnlineSlowLogProvider}
+ */
+ private void confirmPayloadParams(int i, int j, List
slowLogPayloads) {
+Assert.assertEquals(slowLogPayloads.get(i).getClientAddress(), "client_" +
j);
+Assert.assertEquals(slowLogPayloads.get(i).getCallDetails(), "call_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getMethodName(), "method_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getUserName(), "userName_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getProcessingTime(), j + 500);
+Assert.assertEquals(slowLogPayloads.get(i).getQueueTime(), j + 400);
+Assert.assertEquals(slowLogPayloads.get(i).getResponseSize(), j + 200);
+Assert.assertEquals(slowLogPayloads.get(i).getServerClass(), "class_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getParam(), "param_" + j);
+ }
+
+ @Test
+ public void testOnlieSlowLogConsumption() throws Exception {
+Configuration conf = getOnlineSlowLogConf(8);
+onlineSlowLogProvider = new OnlineSlowLogProvider(conf);
+Assert.assertEquals(onlineSlowLogProvider.getSlowLogPayloads().size(), 0);
+LOG.debug("Initially ringbuffer of Slow Log records is empty");
+
+int i = 0;
+
+// add
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377269839
##
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##
@@ -1453,6 +1453,97 @@ def list_decommissioned_regionservers
@admin.listDecommissionedRegionServers
end
+
#--
+# Retrieve SlowLog Responses from RegionServers
+def get_slowlog_responses(server_names, args)
+ unless server_names.is_a?(Array) || server_names.is_a?(String)
+raise(ArgumentError,
+ "#{server_names.class} of #{server_names.inspect} is not of
Array/String type")
+ end
+ if server_names == '*'
+server_names = getServerNames([], true)
+ else
+server_names_list = get_server_names(server_names)
+server_names = getServerNames(server_names_list, false)
+ end
+ slow_log_responses = @admin.getSlowLogResponses(server_names)
+ filtered_log_responses = filter_slow_responses(args,
+ slow_log_responses)
+ puts 'Retrieved SlowLog Responses from RegionServers.'
+ if args.empty?
+puts slow_log_responses.to_s
+ else
+puts filtered_log_responses.to_s
+ end
+end
+
+def filter_slow_responses(args, slow_log_responses)
+ filtered_log_responses = java.util.ArrayList.new
Review comment:
Oh, here's where your filters get applied. Please perform filtering on the
server-side, where it can be distributed across all the region servers
involved, and avoid the network and client overhead.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377262043
##
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/AbstractTestIPC.java
##
@@ -426,4 +437,55 @@ public void testAsyncTimeout() throws IOException {
rpcServer.stop();
}
}
+
+ @Test
Review comment:
Yeah with a void method you're stuck looking for side-effects. Even with a
non-void method, you should still be looking for side-effects of the call...
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log URL: https://github.com/apache/hbase/pull/754#discussion_r377273341 ## File path: src/main/asciidoc/_chapters/hbase-default.adoc ## @@ -2207,3 +2207,39 @@ The percent of region server RPC threads failed to abort RS. .Default `-1` + +[[hbase.regionserver.slowlog.ringbuffer.size]] +*`hbase.regionserver.slowlog.ringbuffer.size`*:: ++ +.Description + + Default size of ringbuffer to be maintained by each RegionServer in order + to store online slowlog responses. This is an in-memory ring buffer of + requests that were judged to be too slow in addition to the responseTooSlow + logging. The in-memory representation would be complete. + For more details: https://issues.apache.org/jira/browse/HBASE-22978 Review comment: Instead of pointing off to a JIRA, how about refer to your new "Get Slow Response Log from shell" section in the book? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377253906
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogEventHandler.java
##
@@ -0,0 +1,137 @@
+/*
+ *
+ * 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.slowlog;
+
+import com.lmax.disruptor.EventHandler;
+import com.lmax.disruptor.RingBuffer;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.stream.Collectors;
+
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Event Handler run by disruptor ringbuffer consumer
+ */
+@InterfaceAudience.Private
+class SlowLogEventHandler implements EventHandler {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(SlowLogEventHandler.class);
+
+ private final SlowLogPayload[] slowLogPayloads;
+
+ private AtomicInteger slowLogPayloadIdx = new AtomicInteger();
+
+ private static final Lock LOCK = new ReentrantLock();
+
+ private static final int WRITE_LOCK_WAIT = 1;
+ private static final int READ_LOCK_WAIT = 1;
+
+ SlowLogEventHandler(int eventCount) {
+this.slowLogPayloads = new SlowLogPayload[eventCount];
+ }
+
+ /**
+ * Called when a publisher has published an event to the {@link RingBuffer}
+ *
+ * @param event published to the {@link RingBuffer}
+ * @param sequence of the event being processed
+ * @param endOfBatch flag to indicate if this is the last event in a batch
from
+ * the {@link RingBuffer}
+ * @throws Exception if the EventHandler would like the exception handled
further up the chain
+ */
+ @Override
+ public void onEvent(RingBufferTruck event, long sequence, boolean
endOfBatch) throws Exception {
+SlowLogPayload slowLogPayload = event.getPayload();
+if (LOG.isTraceEnabled()) {
+ LOG.trace("Received Slow Log Event. RingBuffer sequence: {},
isEndOfBatch: {}, " +
+ "Event Call: {}", sequence, endOfBatch,
+slowLogPayload.getCallDetails());
+}
+try {
+ if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
+int index = slowLogPayloadIdx.getAndSet(
+ (slowLogPayloadIdx.get() + 1) % slowLogPayloads.length);
+slowLogPayloads[index] = slowLogPayload;
+ }
+} finally {
+ LOCK.unlock();
+}
+ }
+
+ /**
+ * Cleans up slow log payloads
+ *
+ * @return true if slow log payloads are cleaned up, false otherwise
+ */
+ boolean clearSlowLogs() {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("Received request to clean up online slowlog buffer..");
+}
+boolean isCleanedUp = true;
Review comment:
You'll return `true` in the event that the lock is never acquired.
Start pessimistic with `isCleanedUp = false` and set it `true` only after
you've successfully done so.
I think this method should look something more like
```java
boolean isCleanedUp = false;
try {
if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
try {
Arrays.fill(slowLogPayloads, null);
slowLogPayloadIdx.set(0);
isCleanedUp = true;
} finally {
LOCK.unlock();
}
} else {
LOG.warn("Failed to acquire write lock while clearing slow logs.");
}
} catch (InterruptedException e) {
LOG.warn("Failed to acquire write lock while clearing slow logs.", e);
Thread.currentThread().interrupt();
}
return isCleanedUp;
```
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.
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377262647
##
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/slowlog/TestOnlineSlowLogProvider.java
##
@@ -0,0 +1,307 @@
+/*
+ *
+ * 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.slowlog;
+
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.Assert;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.util.concurrent.Uninterruptibles;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Tests for Online SlowLog Provider Service
+ */
+@Category({MasterTests.class, MediumTests.class})
+public class TestOnlineSlowLogProvider {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+HBaseClassTestRule.forClass(TestOnlineSlowLogProvider.class);
+
+ private static final Logger LOG =
LoggerFactory.getLogger(TestOnlineSlowLogProvider.class);
+
+ private static final HBaseTestingUtility HBASE_TESTING_UTILITY = new
HBaseTestingUtility();
+
+ private OnlineSlowLogProvider onlineSlowLogProvider;
+
+ private Configuration getOnlineSlowLogConf(int eventSize) {
Review comment:
A `get` method that modifies something under the covers? How about
"applyOnlineSlowLogConf"?
Also, an instance method that modifies static state leads to flakey tests
and limits parallelism. Please refactor.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377270965
##
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##
@@ -1453,6 +1453,97 @@ def list_decommissioned_regionservers
@admin.listDecommissionedRegionServers
end
+
#--
+# Retrieve SlowLog Responses from RegionServers
+def get_slowlog_responses(server_names, args)
+ unless server_names.is_a?(Array) || server_names.is_a?(String)
+raise(ArgumentError,
+ "#{server_names.class} of #{server_names.inspect} is not of
Array/String type")
+ end
+ if server_names == '*'
+server_names = getServerNames([], true)
+ else
+server_names_list = get_server_names(server_names)
+server_names = getServerNames(server_names_list, false)
+ end
+ slow_log_responses = @admin.getSlowLogResponses(server_names)
+ filtered_log_responses = filter_slow_responses(args,
+ slow_log_responses)
+ puts 'Retrieved SlowLog Responses from RegionServers.'
+ if args.empty?
+puts slow_log_responses.to_s
+ else
+puts filtered_log_responses.to_s
+ end
+end
+
+def filter_slow_responses(args, slow_log_responses)
+ filtered_log_responses = java.util.ArrayList.new
+ slow_log_responses.each do |slow_log|
+if args.key? 'REGION_NAME'
+ region_name = args['REGION_NAME']
+ unless region_name.nil?
+if region_name.eql?(slow_log.getRegionName)
+ filtered_log_responses.add(slow_log)
+ next
+end
+ end
+end
+if args.key? 'TABLE_NAME'
+ table_name = args['TABLE_NAME']
+ unless table_name.nil?
+if slow_log.getRegionName.start_with?(table_name)
+ filtered_log_responses.add(slow_log)
+ next
+end
+ end
+end
+if args.key? 'CLIENT_IP'
+ client_ip = args['CLIENT_IP']
+ unless client_ip.nil?
+if client_ip.eql?(slow_log.getClientAddress)
+ filtered_log_responses.add(slow_log)
+ next
+end
+ end
+end
+if args.key? 'USER'
+ user = args['USER']
+ unless user.nil?
+if user.eql?(slow_log.getUserName)
+ filtered_log_responses.add(slow_log)
+ next
+end
+ end
+end
+ end
+ filtered_log_responses
+end
+
+
#--
+# Clears SlowLog Responses from RegionServers
+def clear_slowlog_responses(server_names)
+ unless server_names.nil? || server_names.is_a?(Array) ||
server_names.is_a?(String)
+raise(ArgumentError,
+ "#{server_names.class} of #{server_names.inspect} is not of
correct type")
+ end
+ if server_names.nil?
+server_names = getServerNames([], true)
+ else
+server_names_list = get_server_names(server_names)
+server_names = getServerNames(server_names_list, false)
+ end
+ clear_log_responses = @admin.clearSlowLogResponses(server_names)
+ clear_log_success_count = 0
+ clear_log_responses.each do |response|
+if response
+ clear_log_success_count += 1
+end
+ end
+ puts 'Cleared Slowlog responses from ' \
+ "#{clear_log_success_count}/#{clear_log_responses.size}
RegionServers"
Review comment:
Nice summary. If this isn't 100%, where does the operator go to find out
more? I guess the RS logs, but of which host(s)? We assume all our operators
have a log aggregation tool?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377272626
##
File path:
hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift2/client/ThriftAdmin.java
##
@@ -1157,6 +1158,18 @@ public boolean isSnapshotCleanupEnabled() {
throw new NotImplementedException("isSnapshotCleanupEnabled not supported
in ThriftAdmin");
}
+ @Override
+ public List getSlowLogResponses(final List
serverNames)
+ throws IOException {
+throw new NotImplementedException("getSlowLogResponses not supported in
ThriftAdmin");
Review comment:
Why not? We adding thrift serialization support in a separate patch?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377269081
##
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##
@@ -1453,6 +1453,97 @@ def list_decommissioned_regionservers
@admin.listDecommissionedRegionServers
end
+
#--
+# Retrieve SlowLog Responses from RegionServers
+def get_slowlog_responses(server_names, args)
+ unless server_names.is_a?(Array) || server_names.is_a?(String)
+raise(ArgumentError,
+ "#{server_names.class} of #{server_names.inspect} is not of
Array/String type")
+ end
+ if server_names == '*'
+server_names = getServerNames([], true)
+ else
+server_names_list = get_server_names(server_names)
+server_names = getServerNames(server_names_list, false)
+ end
+ slow_log_responses = @admin.getSlowLogResponses(server_names)
+ filtered_log_responses = filter_slow_responses(args,
+ slow_log_responses)
+ puts 'Retrieved SlowLog Responses from RegionServers.'
Review comment:
nit: What's with the ellipsis everywhere? Here and in log messages, they
don't add anything for the user.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377247844
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/RingBufferTruck.java
##
@@ -0,0 +1,63 @@
+/*
+ *
+ * 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.slowlog;
+
+import org.apache.yetus.audience.InterfaceAudience;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * A truck to carry payload in the slow log ring buffer that serves as online
buffer
+ * to provide latest TooSlowLog
+ */
+@InterfaceAudience.Private
+final class RingBufferTruck {
Review comment:
"Truck" is a bit of an odd name, yeah? What about "Envelope"?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377254804
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogEventHandler.java
##
@@ -0,0 +1,137 @@
+/*
+ *
+ * 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.slowlog;
+
+import com.lmax.disruptor.EventHandler;
+import com.lmax.disruptor.RingBuffer;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.stream.Collectors;
+
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Event Handler run by disruptor ringbuffer consumer
+ */
+@InterfaceAudience.Private
+class SlowLogEventHandler implements EventHandler {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(SlowLogEventHandler.class);
+
+ private final SlowLogPayload[] slowLogPayloads;
+
+ private AtomicInteger slowLogPayloadIdx = new AtomicInteger();
+
+ private static final Lock LOCK = new ReentrantLock();
+
+ private static final int WRITE_LOCK_WAIT = 1;
+ private static final int READ_LOCK_WAIT = 1;
+
+ SlowLogEventHandler(int eventCount) {
+this.slowLogPayloads = new SlowLogPayload[eventCount];
+ }
+
+ /**
+ * Called when a publisher has published an event to the {@link RingBuffer}
+ *
+ * @param event published to the {@link RingBuffer}
+ * @param sequence of the event being processed
+ * @param endOfBatch flag to indicate if this is the last event in a batch
from
+ * the {@link RingBuffer}
+ * @throws Exception if the EventHandler would like the exception handled
further up the chain
+ */
+ @Override
+ public void onEvent(RingBufferTruck event, long sequence, boolean
endOfBatch) throws Exception {
+SlowLogPayload slowLogPayload = event.getPayload();
+if (LOG.isTraceEnabled()) {
+ LOG.trace("Received Slow Log Event. RingBuffer sequence: {},
isEndOfBatch: {}, " +
+ "Event Call: {}", sequence, endOfBatch,
+slowLogPayload.getCallDetails());
+}
+try {
+ if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
+int index = slowLogPayloadIdx.getAndSet(
+ (slowLogPayloadIdx.get() + 1) % slowLogPayloads.length);
+slowLogPayloads[index] = slowLogPayload;
+ }
+} finally {
+ LOCK.unlock();
+}
+ }
+
+ /**
+ * Cleans up slow log payloads
+ *
+ * @return true if slow log payloads are cleaned up, false otherwise
+ */
+ boolean clearSlowLogs() {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("Received request to clean up online slowlog buffer..");
+}
+boolean isCleanedUp = true;
+try {
+ if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
+for (int i = 0; i < slowLogPayloads.length; i++) {
+ slowLogPayloads[i] = null;
+}
+slowLogPayloadIdx.set(0);
+ }
+} catch (InterruptedException e) {
+ isCleanedUp = false;
+ LOG.error("Error while cleaning up slow logs buffer...", e);
+} finally {
+ LOCK.unlock();
+}
+return isCleanedUp;
+ }
+
+ /**
+ * Retrieve list of slow log payloads
+ *
+ * @return list of slow log payloads
+ */
+ List getSlowLogPayloads() {
+List slowLogPayloadList = null;
+try {
+ if (LOCK.tryLock(READ_LOCK_WAIT, TimeUnit.SECONDS)) {
+slowLogPayloadList = Arrays.stream(slowLogPayloads)
+ .filter(Objects::nonNull)
+ .collect(Collectors.toList());
+ }
Review comment:
same lack of `else` clause.
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
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377251146
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogEventHandler.java
##
@@ -0,0 +1,137 @@
+/*
+ *
+ * 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.slowlog;
+
+import com.lmax.disruptor.EventHandler;
+import com.lmax.disruptor.RingBuffer;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.stream.Collectors;
+
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Event Handler run by disruptor ringbuffer consumer
+ */
+@InterfaceAudience.Private
+class SlowLogEventHandler implements EventHandler {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(SlowLogEventHandler.class);
+
+ private final SlowLogPayload[] slowLogPayloads;
+
+ private AtomicInteger slowLogPayloadIdx = new AtomicInteger();
+
+ private static final Lock LOCK = new ReentrantLock();
+
+ private static final int WRITE_LOCK_WAIT = 1;
+ private static final int READ_LOCK_WAIT = 1;
+
+ SlowLogEventHandler(int eventCount) {
+this.slowLogPayloads = new SlowLogPayload[eventCount];
+ }
+
+ /**
+ * Called when a publisher has published an event to the {@link RingBuffer}
+ *
+ * @param event published to the {@link RingBuffer}
+ * @param sequence of the event being processed
+ * @param endOfBatch flag to indicate if this is the last event in a batch
from
+ * the {@link RingBuffer}
+ * @throws Exception if the EventHandler would like the exception handled
further up the chain
+ */
+ @Override
+ public void onEvent(RingBufferTruck event, long sequence, boolean
endOfBatch) throws Exception {
+SlowLogPayload slowLogPayload = event.getPayload();
+if (LOG.isTraceEnabled()) {
+ LOG.trace("Received Slow Log Event. RingBuffer sequence: {},
isEndOfBatch: {}, " +
+ "Event Call: {}", sequence, endOfBatch,
+slowLogPayload.getCallDetails());
+}
+try {
+ if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
+int index = slowLogPayloadIdx.getAndSet(
+ (slowLogPayloadIdx.get() + 1) % slowLogPayloads.length);
Review comment:
Calling `get()` from within `getAndSet()` defeats the atomicity of an
`AtomicInteger`, yeah? I think you meant to write
```java
final int index = slowLogPayloadIdx.getAndUpdate(val -> (val + 1) %
slowLogPayloads.length);
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377267771
##
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/slowlog/TestOnlineSlowLogProvider.java
##
@@ -0,0 +1,307 @@
+/*
+ *
+ * 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.slowlog;
+
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.Assert;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.util.concurrent.Uninterruptibles;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Tests for Online SlowLog Provider Service
+ */
+@Category({MasterTests.class, MediumTests.class})
+public class TestOnlineSlowLogProvider {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+HBaseClassTestRule.forClass(TestOnlineSlowLogProvider.class);
+
+ private static final Logger LOG =
LoggerFactory.getLogger(TestOnlineSlowLogProvider.class);
+
+ private static final HBaseTestingUtility HBASE_TESTING_UTILITY = new
HBaseTestingUtility();
+
+ private OnlineSlowLogProvider onlineSlowLogProvider;
+
+ private Configuration getOnlineSlowLogConf(int eventSize) {
+Configuration conf = HBASE_TESTING_UTILITY.getConfiguration();
+conf.setBoolean(HConstants.SLOW_LOG_BUFFER_ENABLED_KEY, true);
+conf.setInt(HConstants.SLOW_LOG_RING_BUFFER_SIZE, eventSize);
+return conf;
+ }
+
+ private SlowLogPayload getSlowLogPayload(int i) {
+SlowLogPayload slowLogPayload = SlowLogPayload.newBuilder()
+ .setCallDetails("call_" + i)
+ .setClientAddress("client_" + i)
+ .setMethodName("method_" + i)
+ .setUserName("userName_" + i)
+ .setMultiGets(i)
+ .setMultiMutations(i)
+ .setMultiServiceCalls(i)
+ .setProcessingTime(i + 500)
+ .setQueueTime(i + 400)
+ .setResponseSize(i + 200)
+ .setStartTime(EnvironmentEdgeManager.currentTime())
+ .setServerClass("class_" + i)
+ .setParam("param_" + i)
+ .build();
+return slowLogPayload;
+ }
+
+ /**
+ * confirm that for a ringbuffer of slow logs, payload on given index of
buffer
+ * has expected elements
+ *
+ * @param i index of ringbuffer logs
+ * @param j data value that was put on index i
+ * @param slowLogPayloads list of payload retrieved from {@link
OnlineSlowLogProvider}
+ */
+ private void confirmPayloadParams(int i, int j, List
slowLogPayloads) {
+Assert.assertEquals(slowLogPayloads.get(i).getClientAddress(), "client_" +
j);
+Assert.assertEquals(slowLogPayloads.get(i).getCallDetails(), "call_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getMethodName(), "method_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getUserName(), "userName_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getProcessingTime(), j + 500);
+Assert.assertEquals(slowLogPayloads.get(i).getQueueTime(), j + 400);
+Assert.assertEquals(slowLogPayloads.get(i).getResponseSize(), j + 200);
+Assert.assertEquals(slowLogPayloads.get(i).getServerClass(), "class_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getParam(), "param_" + j);
+ }
+
+ @Test
+ public void testOnlieSlowLogConsumption() throws Exception {
+Configuration conf = getOnlineSlowLogConf(8);
+onlineSlowLogProvider = new OnlineSlowLogProvider(conf);
+Assert.assertEquals(onlineSlowLogProvider.getSlowLogPayloads().size(), 0);
+LOG.debug("Initially ringbuffer of Slow Log records is empty");
+
+int i = 0;
+
+// add
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377267219
##
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/slowlog/TestOnlineSlowLogProvider.java
##
@@ -0,0 +1,307 @@
+/*
+ *
+ * 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.slowlog;
+
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.Assert;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.util.concurrent.Uninterruptibles;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Tests for Online SlowLog Provider Service
+ */
+@Category({MasterTests.class, MediumTests.class})
+public class TestOnlineSlowLogProvider {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+HBaseClassTestRule.forClass(TestOnlineSlowLogProvider.class);
+
+ private static final Logger LOG =
LoggerFactory.getLogger(TestOnlineSlowLogProvider.class);
+
+ private static final HBaseTestingUtility HBASE_TESTING_UTILITY = new
HBaseTestingUtility();
+
+ private OnlineSlowLogProvider onlineSlowLogProvider;
+
+ private Configuration getOnlineSlowLogConf(int eventSize) {
+Configuration conf = HBASE_TESTING_UTILITY.getConfiguration();
+conf.setBoolean(HConstants.SLOW_LOG_BUFFER_ENABLED_KEY, true);
+conf.setInt(HConstants.SLOW_LOG_RING_BUFFER_SIZE, eventSize);
+return conf;
+ }
+
+ private SlowLogPayload getSlowLogPayload(int i) {
+SlowLogPayload slowLogPayload = SlowLogPayload.newBuilder()
+ .setCallDetails("call_" + i)
+ .setClientAddress("client_" + i)
+ .setMethodName("method_" + i)
+ .setUserName("userName_" + i)
+ .setMultiGets(i)
+ .setMultiMutations(i)
+ .setMultiServiceCalls(i)
+ .setProcessingTime(i + 500)
+ .setQueueTime(i + 400)
+ .setResponseSize(i + 200)
+ .setStartTime(EnvironmentEdgeManager.currentTime())
+ .setServerClass("class_" + i)
+ .setParam("param_" + i)
+ .build();
+return slowLogPayload;
+ }
+
+ /**
+ * confirm that for a ringbuffer of slow logs, payload on given index of
buffer
+ * has expected elements
+ *
+ * @param i index of ringbuffer logs
+ * @param j data value that was put on index i
+ * @param slowLogPayloads list of payload retrieved from {@link
OnlineSlowLogProvider}
+ */
+ private void confirmPayloadParams(int i, int j, List
slowLogPayloads) {
+Assert.assertEquals(slowLogPayloads.get(i).getClientAddress(), "client_" +
j);
+Assert.assertEquals(slowLogPayloads.get(i).getCallDetails(), "call_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getMethodName(), "method_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getUserName(), "userName_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getProcessingTime(), j + 500);
+Assert.assertEquals(slowLogPayloads.get(i).getQueueTime(), j + 400);
+Assert.assertEquals(slowLogPayloads.get(i).getResponseSize(), j + 200);
+Assert.assertEquals(slowLogPayloads.get(i).getServerClass(), "class_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getParam(), "param_" + j);
+ }
+
+ @Test
+ public void testOnlieSlowLogConsumption() throws Exception {
+Configuration conf = getOnlineSlowLogConf(8);
+onlineSlowLogProvider = new OnlineSlowLogProvider(conf);
+Assert.assertEquals(onlineSlowLogProvider.getSlowLogPayloads().size(), 0);
+LOG.debug("Initially ringbuffer of Slow Log records is empty");
+
+int i = 0;
+
+// add
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377258606
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogEventHandler.java
##
@@ -0,0 +1,137 @@
+/*
+ *
+ * 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.slowlog;
+
+import com.lmax.disruptor.EventHandler;
+import com.lmax.disruptor.RingBuffer;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.stream.Collectors;
+
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Event Handler run by disruptor ringbuffer consumer
+ */
+@InterfaceAudience.Private
+class SlowLogEventHandler implements EventHandler {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(SlowLogEventHandler.class);
+
+ private final SlowLogPayload[] slowLogPayloads;
+
+ private AtomicInteger slowLogPayloadIdx = new AtomicInteger();
+
+ private static final Lock LOCK = new ReentrantLock();
+
+ private static final int WRITE_LOCK_WAIT = 1;
+ private static final int READ_LOCK_WAIT = 1;
+
+ SlowLogEventHandler(int eventCount) {
+this.slowLogPayloads = new SlowLogPayload[eventCount];
+ }
+
+ /**
+ * Called when a publisher has published an event to the {@link RingBuffer}
+ *
+ * @param event published to the {@link RingBuffer}
+ * @param sequence of the event being processed
+ * @param endOfBatch flag to indicate if this is the last event in a batch
from
+ * the {@link RingBuffer}
+ * @throws Exception if the EventHandler would like the exception handled
further up the chain
+ */
+ @Override
+ public void onEvent(RingBufferTruck event, long sequence, boolean
endOfBatch) throws Exception {
+SlowLogPayload slowLogPayload = event.getPayload();
+if (LOG.isTraceEnabled()) {
+ LOG.trace("Received Slow Log Event. RingBuffer sequence: {},
isEndOfBatch: {}, " +
+ "Event Call: {}", sequence, endOfBatch,
+slowLogPayload.getCallDetails());
+}
+try {
+ if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
+int index = slowLogPayloadIdx.getAndSet(
+ (slowLogPayloadIdx.get() + 1) % slowLogPayloads.length);
+slowLogPayloads[index] = slowLogPayload;
+ }
+} finally {
+ LOCK.unlock();
Review comment:
What happens when `unlock` is called by `tryLock()` returned `false`? Is it
a no-op or does it throw `IllegalMonitorStateException` ?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377264208
##
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/slowlog/TestOnlineSlowLogProvider.java
##
@@ -0,0 +1,307 @@
+/*
+ *
+ * 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.slowlog;
+
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.Assert;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.util.concurrent.Uninterruptibles;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Tests for Online SlowLog Provider Service
+ */
+@Category({MasterTests.class, MediumTests.class})
+public class TestOnlineSlowLogProvider {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+HBaseClassTestRule.forClass(TestOnlineSlowLogProvider.class);
+
+ private static final Logger LOG =
LoggerFactory.getLogger(TestOnlineSlowLogProvider.class);
+
+ private static final HBaseTestingUtility HBASE_TESTING_UTILITY = new
HBaseTestingUtility();
+
+ private OnlineSlowLogProvider onlineSlowLogProvider;
+
+ private Configuration getOnlineSlowLogConf(int eventSize) {
+Configuration conf = HBASE_TESTING_UTILITY.getConfiguration();
+conf.setBoolean(HConstants.SLOW_LOG_BUFFER_ENABLED_KEY, true);
+conf.setInt(HConstants.SLOW_LOG_RING_BUFFER_SIZE, eventSize);
+return conf;
+ }
+
+ private SlowLogPayload getSlowLogPayload(int i) {
+SlowLogPayload slowLogPayload = SlowLogPayload.newBuilder()
+ .setCallDetails("call_" + i)
+ .setClientAddress("client_" + i)
+ .setMethodName("method_" + i)
+ .setUserName("userName_" + i)
+ .setMultiGets(i)
+ .setMultiMutations(i)
+ .setMultiServiceCalls(i)
+ .setProcessingTime(i + 500)
+ .setQueueTime(i + 400)
+ .setResponseSize(i + 200)
+ .setStartTime(EnvironmentEdgeManager.currentTime())
+ .setServerClass("class_" + i)
+ .setParam("param_" + i)
+ .build();
+return slowLogPayload;
+ }
+
+ /**
+ * confirm that for a ringbuffer of slow logs, payload on given index of
buffer
+ * has expected elements
+ *
+ * @param i index of ringbuffer logs
+ * @param j data value that was put on index i
+ * @param slowLogPayloads list of payload retrieved from {@link
OnlineSlowLogProvider}
+ */
+ private void confirmPayloadParams(int i, int j, List
slowLogPayloads) {
+Assert.assertEquals(slowLogPayloads.get(i).getClientAddress(), "client_" +
j);
+Assert.assertEquals(slowLogPayloads.get(i).getCallDetails(), "call_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getMethodName(), "method_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getUserName(), "userName_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getProcessingTime(), j + 500);
+Assert.assertEquals(slowLogPayloads.get(i).getQueueTime(), j + 400);
+Assert.assertEquals(slowLogPayloads.get(i).getResponseSize(), j + 200);
+Assert.assertEquals(slowLogPayloads.get(i).getServerClass(), "class_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getParam(), "param_" + j);
+ }
+
+ @Test
+ public void testOnlieSlowLogConsumption() throws Exception {
+Configuration conf = getOnlineSlowLogConf(8);
+onlineSlowLogProvider = new OnlineSlowLogProvider(conf);
+Assert.assertEquals(onlineSlowLogProvider.getSlowLogPayloads().size(), 0);
+LOG.debug("Initially ringbuffer of Slow Log records is empty");
+
+int i = 0;
+
+// add
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377259497
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogEventHandler.java
##
@@ -0,0 +1,137 @@
+/*
+ *
+ * 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.slowlog;
+
+import com.lmax.disruptor.EventHandler;
+import com.lmax.disruptor.RingBuffer;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.stream.Collectors;
+
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Event Handler run by disruptor ringbuffer consumer
+ */
+@InterfaceAudience.Private
+class SlowLogEventHandler implements EventHandler {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(SlowLogEventHandler.class);
+
+ private final SlowLogPayload[] slowLogPayloads;
+
+ private AtomicInteger slowLogPayloadIdx = new AtomicInteger();
+
+ private static final Lock LOCK = new ReentrantLock();
+
+ private static final int WRITE_LOCK_WAIT = 1;
+ private static final int READ_LOCK_WAIT = 1;
+
+ SlowLogEventHandler(int eventCount) {
+this.slowLogPayloads = new SlowLogPayload[eventCount];
+ }
+
+ /**
+ * Called when a publisher has published an event to the {@link RingBuffer}
+ *
+ * @param event published to the {@link RingBuffer}
+ * @param sequence of the event being processed
+ * @param endOfBatch flag to indicate if this is the last event in a batch
from
+ * the {@link RingBuffer}
+ * @throws Exception if the EventHandler would like the exception handled
further up the chain
+ */
+ @Override
+ public void onEvent(RingBufferTruck event, long sequence, boolean
endOfBatch) throws Exception {
+SlowLogPayload slowLogPayload = event.getPayload();
+if (LOG.isTraceEnabled()) {
+ LOG.trace("Received Slow Log Event. RingBuffer sequence: {},
isEndOfBatch: {}, " +
+ "Event Call: {}", sequence, endOfBatch,
+slowLogPayload.getCallDetails());
+}
+try {
+ if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
+int index = slowLogPayloadIdx.getAndSet(
+ (slowLogPayloadIdx.get() + 1) % slowLogPayloads.length);
+slowLogPayloads[index] = slowLogPayload;
+ }
+} finally {
+ LOCK.unlock();
+}
+ }
+
+ /**
+ * Cleans up slow log payloads
+ *
+ * @return true if slow log payloads are cleaned up, false otherwise
+ */
+ boolean clearSlowLogs() {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("Received request to clean up online slowlog buffer..");
+}
+boolean isCleanedUp = true;
+try {
+ if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
+for (int i = 0; i < slowLogPayloads.length; i++) {
+ slowLogPayloads[i] = null;
+}
+slowLogPayloadIdx.set(0);
+ }
+} catch (InterruptedException e) {
+ isCleanedUp = false;
+ LOG.error("Error while cleaning up slow logs buffer...", e);
+} finally {
+ LOCK.unlock();
+}
+return isCleanedUp;
+ }
+
+ /**
+ * Retrieve list of slow log payloads
+ *
+ * @return list of slow log payloads
+ */
+ List getSlowLogPayloads() {
+List slowLogPayloadList = null;
+try {
+ if (LOCK.tryLock(READ_LOCK_WAIT, TimeUnit.SECONDS)) {
+slowLogPayloadList = Arrays.stream(slowLogPayloads)
+ .filter(Objects::nonNull)
+ .collect(Collectors.toList());
+ }
+} catch (InterruptedException e) {
+ LOG.error("Error while reading slow logs buffer...", e);
Review comment:
Is swallowing the interrupt the right thing to do? Someone is requesting
this thread terminate...
This is an
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377252887
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogEventHandler.java
##
@@ -0,0 +1,137 @@
+/*
+ *
+ * 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.slowlog;
+
+import com.lmax.disruptor.EventHandler;
+import com.lmax.disruptor.RingBuffer;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.stream.Collectors;
+
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Event Handler run by disruptor ringbuffer consumer
+ */
+@InterfaceAudience.Private
+class SlowLogEventHandler implements EventHandler {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(SlowLogEventHandler.class);
+
+ private final SlowLogPayload[] slowLogPayloads;
+
+ private AtomicInteger slowLogPayloadIdx = new AtomicInteger();
+
+ private static final Lock LOCK = new ReentrantLock();
+
+ private static final int WRITE_LOCK_WAIT = 1;
+ private static final int READ_LOCK_WAIT = 1;
+
+ SlowLogEventHandler(int eventCount) {
+this.slowLogPayloads = new SlowLogPayload[eventCount];
+ }
+
+ /**
+ * Called when a publisher has published an event to the {@link RingBuffer}
+ *
+ * @param event published to the {@link RingBuffer}
+ * @param sequence of the event being processed
+ * @param endOfBatch flag to indicate if this is the last event in a batch
from
+ * the {@link RingBuffer}
+ * @throws Exception if the EventHandler would like the exception handled
further up the chain
+ */
+ @Override
+ public void onEvent(RingBufferTruck event, long sequence, boolean
endOfBatch) throws Exception {
+SlowLogPayload slowLogPayload = event.getPayload();
+if (LOG.isTraceEnabled()) {
+ LOG.trace("Received Slow Log Event. RingBuffer sequence: {},
isEndOfBatch: {}, " +
+ "Event Call: {}", sequence, endOfBatch,
+slowLogPayload.getCallDetails());
+}
+try {
+ if (LOCK.tryLock(WRITE_LOCK_WAIT, TimeUnit.SECONDS)) {
+int index = slowLogPayloadIdx.getAndSet(
+ (slowLogPayloadIdx.get() + 1) % slowLogPayloads.length);
+slowLogPayloads[index] = slowLogPayload;
+ }
Review comment:
what about the `false` condition? Just ignore the event? Seems like the
wrong thing to do.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377261014
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/OnlineSlowLogProvider.java
##
@@ -0,0 +1,137 @@
+/**
+ *
+ * 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.slowlog;
+
+import com.lmax.disruptor.BlockingWaitStrategy;
+import com.lmax.disruptor.RingBuffer;
+import com.lmax.disruptor.dsl.Disruptor;
+import com.lmax.disruptor.dsl.ProducerType;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.util.Threads;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.yetus.audience.InterfaceStability;
+
+import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Online SlowLog Provider Service
+ */
+@InterfaceAudience.Private
+@InterfaceStability.Evolving
+public class OnlineSlowLogProvider {
Review comment:
This class doesn't provide SlowLogs, and I'm unclear on what makes it
online. How about naming it `SlowLogLogger` or `SlowLogRecorder` or
`SlowLogHandlingService` ? I guess others don't like the name "Service..."
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377266290
##
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/slowlog/TestOnlineSlowLogProvider.java
##
@@ -0,0 +1,307 @@
+/*
+ *
+ * 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.slowlog;
+
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.Assert;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.util.concurrent.Uninterruptibles;
+
+import
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload;
+
+/**
+ * Tests for Online SlowLog Provider Service
+ */
+@Category({MasterTests.class, MediumTests.class})
+public class TestOnlineSlowLogProvider {
+
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+HBaseClassTestRule.forClass(TestOnlineSlowLogProvider.class);
+
+ private static final Logger LOG =
LoggerFactory.getLogger(TestOnlineSlowLogProvider.class);
+
+ private static final HBaseTestingUtility HBASE_TESTING_UTILITY = new
HBaseTestingUtility();
+
+ private OnlineSlowLogProvider onlineSlowLogProvider;
+
+ private Configuration getOnlineSlowLogConf(int eventSize) {
+Configuration conf = HBASE_TESTING_UTILITY.getConfiguration();
+conf.setBoolean(HConstants.SLOW_LOG_BUFFER_ENABLED_KEY, true);
+conf.setInt(HConstants.SLOW_LOG_RING_BUFFER_SIZE, eventSize);
+return conf;
+ }
+
+ private SlowLogPayload getSlowLogPayload(int i) {
+SlowLogPayload slowLogPayload = SlowLogPayload.newBuilder()
+ .setCallDetails("call_" + i)
+ .setClientAddress("client_" + i)
+ .setMethodName("method_" + i)
+ .setUserName("userName_" + i)
+ .setMultiGets(i)
+ .setMultiMutations(i)
+ .setMultiServiceCalls(i)
+ .setProcessingTime(i + 500)
+ .setQueueTime(i + 400)
+ .setResponseSize(i + 200)
+ .setStartTime(EnvironmentEdgeManager.currentTime())
+ .setServerClass("class_" + i)
+ .setParam("param_" + i)
+ .build();
+return slowLogPayload;
+ }
+
+ /**
+ * confirm that for a ringbuffer of slow logs, payload on given index of
buffer
+ * has expected elements
+ *
+ * @param i index of ringbuffer logs
+ * @param j data value that was put on index i
+ * @param slowLogPayloads list of payload retrieved from {@link
OnlineSlowLogProvider}
+ */
+ private void confirmPayloadParams(int i, int j, List
slowLogPayloads) {
+Assert.assertEquals(slowLogPayloads.get(i).getClientAddress(), "client_" +
j);
+Assert.assertEquals(slowLogPayloads.get(i).getCallDetails(), "call_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getMethodName(), "method_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getUserName(), "userName_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getProcessingTime(), j + 500);
+Assert.assertEquals(slowLogPayloads.get(i).getQueueTime(), j + 400);
+Assert.assertEquals(slowLogPayloads.get(i).getResponseSize(), j + 200);
+Assert.assertEquals(slowLogPayloads.get(i).getServerClass(), "class_" + j);
+Assert.assertEquals(slowLogPayloads.get(i).getParam(), "param_" + j);
+ }
+
+ @Test
+ public void testOnlieSlowLogConsumption() throws Exception {
+Configuration conf = getOnlineSlowLogConf(8);
+onlineSlowLogProvider = new OnlineSlowLogProvider(conf);
+Assert.assertEquals(onlineSlowLogProvider.getSlowLogPayloads().size(), 0);
+LOG.debug("Initially ringbuffer of Slow Log records is empty");
+
+int i = 0;
+
+// add
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377235034
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
##
@@ -496,11 +518,51 @@ void logResponse(Message param, String methodName,
String call, String tag,
}
}
}
- responseInfo.put("multi.gets", numGets);
- responseInfo.put("multi.mutations", numMutations);
- responseInfo.put("multi.servicecalls", numServiceCalls);
+ responseInfo.put(MULTI_GETS, numGets);
+ responseInfo.put(MULTI_MUTATIONS, numMutations);
+ responseInfo.put(MULTI_SERVICE_CALLS, numServiceCalls);
}
+final String tag = tooLarge ? "TooLarge" : "TooSlow";
Review comment:
Again, "too large" and "too slow" are not mutually exclusive. Could be a
message is logged with both tags, right?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377230115
##
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java
##
@@ -3368,4 +3429,56 @@ public static RegionStatesCount
toTableRegionStatesCount(
.build();
}
+ /**
+ * Convert Protobuf class
+ * {@link
org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog.SlowLogPayload}
+ * To client SlowLog Payload class {@link SlowLogPayload}
+ *
+ * @param slowLogPayload SlowLog Payload protobuf instance
+ * @return SlowLog Payload for client usecase
+ */
+ private static SlowLogPayload getSlowLogPayload(
+ final TooSlowLog.SlowLogPayload slowLogPayload) {
+SlowLogPayload clientSlowLogPayload = new
SlowLogPayload.SlowLogPayloadBuilder()
+ .setStartTime(slowLogPayload.getStartTime())
Review comment:
nit: maybe order these alphabetically so it's easier to spot a missing field.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377236813
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
##
@@ -496,11 +518,51 @@ void logResponse(Message param, String methodName,
String call, String tag,
}
}
}
- responseInfo.put("multi.gets", numGets);
- responseInfo.put("multi.mutations", numMutations);
- responseInfo.put("multi.servicecalls", numServiceCalls);
+ responseInfo.put(MULTI_GETS, numGets);
+ responseInfo.put(MULTI_MUTATIONS, numMutations);
+ responseInfo.put(MULTI_SERVICE_CALLS, numServiceCalls);
}
+final String tag = tooLarge ? "TooLarge" : "TooSlow";
LOG.warn("(response" + tag + "): " + GSON.toJson(responseInfo));
+if (tooSlow && this.onlineSlowLogProvider != null
+&& !call.startsWith(GET_SLOW_LOG_RESPONSES)
+&& !call.startsWith(CLEAR_SLOW_LOGS_RESPONSES)) {
+ logOnlineSlowResponse(param, methodName, call, clientAddress, startTime,
+processingTime, qTime, responseSize, userName, className,
responseInfo);
+}
+ }
+
+ private void logOnlineSlowResponse(Message param, String methodName, String
call,
+ String clientAddress, long startTime, int processingTime, int qTime,
long responseSize,
+ String userName, String className, Map responseInfo) {
+// add too slow log to ringbuffer for retrieval of latest n slow logs
+
+try {
+ final SlowLogParams slowLogParams = ProtobufUtil.getSlowLogParams(param);
+
+ final SlowLogPayload slowLogPayload = SlowLogPayload.newBuilder()
+.setStartTime(startTime)
Review comment:
same comment re: ordering these alphabetically so that it's easier to spot a
missed field.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow response log
ndimiduk commented on a change in pull request #754: HBASE-22978 : Online slow
response log
URL: https://github.com/apache/hbase/pull/754#discussion_r377242097
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
##
@@ -590,6 +596,10 @@ public HRegionServer(final Configuration conf) throws
IOException {
this.abortRequested = false;
this.stopped = false;
+ // initiate online slowlog ringbuffer only for RegionServers
+ if (!(this instanceof HMaster)) {
Review comment:
I agree that things are a bit of a mess currently. I was hoping that, with a
little prodding, you might see an obvious better way ;)
I agree that checking the instance type is better than `getProcessName`.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] ndimiduk opened a new pull request #1160: HBASE-23802 Remove unnecessary Configuration instantiation in LossyAc…
ndimiduk opened a new pull request #1160: HBASE-23802 Remove unnecessary Configuration instantiation in LossyAc… URL: https://github.com/apache/hbase/pull/1160 …counting (#1127) Signed-off-by: stack 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] ndimiduk commented on a change in pull request #1137: HBASE-23804: Fix default master addr hostname in master registry
ndimiduk commented on a change in pull request #1137: HBASE-23804: Fix default master addr hostname in master registry URL: https://github.com/apache/hbase/pull/1137#discussion_r377340652 ## File path: hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java ## @@ -181,7 +181,11 @@ /** Configuration key for the list of master host:ports **/ public static final String MASTER_ADDRS_KEY = "hbase.masters"; - public static final String MASTER_ADDRS_DEFAULT = "localhost:" + DEFAULT_MASTER_PORT; Review comment: Ah okay. Let's avoid adding any fields to `HConstants` on the feature branch if we can. 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Comment Edited] (HBASE-23825) Increment proto conversion is broken
[ https://issues.apache.org/jira/browse/HBASE-23825?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033988#comment-17033988 ] Abhishek Singh Chouhan edited comment on HBASE-23825 at 2/10/20 10:16 PM: -- This is not a problem in master and 2.x since we reverted HBASE-18026 from those branches. FYI [~andrew.purt...@gmail.com] was (Author: abhishek.chouhan): This is not a problem in master and 2.x since we reverted HBASE-18026. FYI [~andrew.purt...@gmail.com] > Increment proto conversion is broken > > > Key: HBASE-23825 > URL: https://issues.apache.org/jira/browse/HBASE-23825 > Project: HBase > Issue Type: Bug >Affects Versions: 1.5.0, 1.3.6, 1.4.12 >Reporter: Abhishek Singh Chouhan >Assignee: Abhishek Singh Chouhan >Priority: Major > > While converting the request back to Increment using ProtobufUtil.toIncrement > we incorrectly use the optimization to avoid copying the byte > array(HBaseZeroCopyByteString#zeroCopyGetBytes) on a BoundedByteString. The > optimization was only meant for LiteralByteString where it is safe to use the > backing byte array, however it ends up being used to BoundedByteString which > is a subclass of LiteralByteString. This essentially breaks increments since > we end up creating wrong cells on the server side. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (HBASE-23825) Increment proto conversion is broken
[ https://issues.apache.org/jira/browse/HBASE-23825?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033988#comment-17033988 ] Abhishek Singh Chouhan commented on HBASE-23825: This is not a problem in master and 2.x since we reverted HBASE-18026. FYI [~andrew.purt...@gmail.com] > Increment proto conversion is broken > > > Key: HBASE-23825 > URL: https://issues.apache.org/jira/browse/HBASE-23825 > Project: HBase > Issue Type: Bug >Affects Versions: 1.5.0, 1.3.6, 1.4.12 >Reporter: Abhishek Singh Chouhan >Assignee: Abhishek Singh Chouhan >Priority: Major > > While converting the request back to Increment using ProtobufUtil.toIncrement > we incorrectly use the optimization to avoid copying the byte > array(HBaseZeroCopyByteString#zeroCopyGetBytes) on a BoundedByteString. The > optimization was only meant for LiteralByteString where it is safe to use the > backing byte array, however it ends up being used to BoundedByteString which > is a subclass of LiteralByteString. This essentially breaks increments since > we end up creating wrong cells on the server side. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [hbase] Apache-HBase commented on issue #1158: HBASE-23823 Run maven with more than default single thread (--threads…
Apache-HBase commented on issue #1158: HBASE-23823 Run maven with more than default single thread (--threads… URL: https://github.com/apache/hbase/pull/1158#issuecomment-584398591 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Comment | |::|--:|:|:| | +0 :ok: | reexec | 1m 7s | Docker mode activated. | ||| _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. | | +0 :ok: | shelldocs | 0m 0s | Shelldocs was not available. | | +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. | | -0 :warning: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. | ||| _ branch-2 Compile Tests _ | | +1 :green_heart: | mvninstall | 5m 57s | branch-2 passed | | +1 :green_heart: | compile | 3m 19s | branch-2 passed | | +1 :green_heart: | shadedjars | 4m 42s | branch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | javadoc | 2m 55s | branch-2 passed | ||| _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 5m 26s | the patch passed | | +1 :green_heart: | compile | 3m 18s | the patch passed | | +1 :green_heart: | javac | 3m 18s | the patch passed | | +1 :green_heart: | shellcheck | 0m 4s | There were no new shellcheck issues. | | +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. | | +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. | | +1 :green_heart: | shadedjars | 4m 43s | patch has no errors when building our shaded downstream artifacts. | | +1 :green_heart: | hadoopcheck | 17m 20s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. | | +1 :green_heart: | javadoc | 2m 54s | the patch passed | ||| _ Other Tests _ | | -1 :x: | unit | 106m 9s | root in the patch failed. | | +1 :green_heart: | asflicense | 0m 33s | The patch does not generate ASF License warnings. | | | | 165m 14s | | | Reason | Tests | |---:|:--| | Failed junit tests | hadoop.hbase.master.assignment.TestCloseRegionWhileRSCrash | | Subsystem | Report/Notes | |--:|:-| | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1158/1/artifact/out/Dockerfile | | GITHUB PR | https://github.com/apache/hbase/pull/1158 | | Optional Tests | dupname asflicense shellcheck shelldocs javac javadoc unit shadedjars hadoopcheck xml compile | | uname | Linux ae4fe6f6e845 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 GNU/Linux | | Build tool | maven | | Personality | /home/jenkins/jenkins-slave/workspace/Base-PreCommit-GitHub-PR_PR-1158/out/precommit/personality/provided.sh | | git revision | branch-2 / 50eaa0293e | | Default Java | 1.8.0_181 | | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1158/1/artifact/out/patch-unit-root.txt | | Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1158/1/testReport/ | | Max. process+thread count | 5934 (vs. ulimit of 1) | | modules | C: . U: . | | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1158/1/console | | versions | git=2.11.0 maven=2018-06-17T18:33:14Z) shellcheck=0.7.0 | | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org | This message was automatically generated. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377366112
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCompactionChore.java
##
@@ -0,0 +1,253 @@
+/**
+ * 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.mob;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ScheduledChore;
+import org.apache.hadoop.hbase.TableDescriptors;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.CompactionState;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableState;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+
+
+/**
+ * Periodic MOB compaction chore.
+ * It runs MOB compaction on region servers in parallel, thus
+ * utilizing distributed cluster resources. To avoid possible major
+ * compaction storms, one can specify maximum number regions to be compacted
+ * in parallel by setting configuration parameter:
+ * 'hbase.mob.major.compaction.region.batch.size', which by default is 0
(unlimited).
+ *
+ */
+@InterfaceAudience.Private
+public class MobFileCompactionChore extends ScheduledChore {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MobFileCompactionChore.class);
+ private Configuration conf;
+ private HMaster master;
+ private int regionBatchSize = 0;// not set - compact all
+
+ public MobFileCompactionChore(HMaster master) {
+super(master.getServerName() + "-MobFileCompactionChore", master,
+
master.getConfiguration().getInt(MobConstants.MOB_COMPACTION_CHORE_PERIOD,
+ MobConstants.DEFAULT_MOB_COMPACTION_CHORE_PERIOD),
+
master.getConfiguration().getInt(MobConstants.MOB_COMPACTION_CHORE_PERIOD,
+ MobConstants.DEFAULT_MOB_COMPACTION_CHORE_PERIOD),
+TimeUnit.SECONDS);
+this.master = master;
+this.conf = master.getConfiguration();
+this.regionBatchSize =
+
master.getConfiguration().getInt(MobConstants.MOB_MAJOR_COMPACTION_REGION_BATCH_SIZE,
+ MobConstants.DEFAULT_MOB_MAJOR_COMPACTION_REGION_BATCH_SIZE);
+
+ }
+
+ @VisibleForTesting
+ public MobFileCompactionChore(Configuration conf, int batchSize) {
+this.conf = conf;
+this.regionBatchSize = batchSize;
+ }
+
+ @Override
+ protected void chore() {
+
+boolean reported = false;
+
+try (Connection conn = ConnectionFactory.createConnection(conf);
+Admin admin = conn.getAdmin();) {
+
+ TableDescriptors htds = master.getTableDescriptors();
+ Map map = htds.getAll();
+ for (TableDescriptor htd : map.values()) {
+if (!master.getTableStateManager().isTableState(htd.getTableName(),
+ TableState.State.ENABLED)) {
+ LOG.info("Skipping MOB compaction on table {} because it is not
ENABLED",
+htd.getTableName());
+ continue;
+} else {
+ LOG.info("Starting MOB compaction on table {}, checking {} column
families",
+htd.getTableName(), htd.getColumnFamilyCount());
+}
+for (ColumnFamilyDescriptor hcd : htd.getColumnFamilies()) {
+ try {
+if (hcd.isMobEnabled()) {
+ if (!reported) {
+master.reportMobCompactionStart(htd.getTableName());
+reported = true;
+ }
+ LOG.info("Major
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377366253
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCompactionChore.java
##
@@ -0,0 +1,253 @@
+/**
+ * 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.mob;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ScheduledChore;
+import org.apache.hadoop.hbase.TableDescriptors;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.CompactionState;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.RegionInfo;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableState;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+
+
+/**
+ * Periodic MOB compaction chore.
+ * It runs MOB compaction on region servers in parallel, thus
+ * utilizing distributed cluster resources. To avoid possible major
+ * compaction storms, one can specify maximum number regions to be compacted
+ * in parallel by setting configuration parameter:
+ * 'hbase.mob.major.compaction.region.batch.size', which by default is 0
(unlimited).
+ *
+ */
+@InterfaceAudience.Private
+public class MobFileCompactionChore extends ScheduledChore {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MobFileCompactionChore.class);
+ private Configuration conf;
+ private HMaster master;
+ private int regionBatchSize = 0;// not set - compact all
+
+ public MobFileCompactionChore(HMaster master) {
+super(master.getServerName() + "-MobFileCompactionChore", master,
+
master.getConfiguration().getInt(MobConstants.MOB_COMPACTION_CHORE_PERIOD,
+ MobConstants.DEFAULT_MOB_COMPACTION_CHORE_PERIOD),
+
master.getConfiguration().getInt(MobConstants.MOB_COMPACTION_CHORE_PERIOD,
+ MobConstants.DEFAULT_MOB_COMPACTION_CHORE_PERIOD),
+TimeUnit.SECONDS);
+this.master = master;
+this.conf = master.getConfiguration();
+this.regionBatchSize =
+
master.getConfiguration().getInt(MobConstants.MOB_MAJOR_COMPACTION_REGION_BATCH_SIZE,
+ MobConstants.DEFAULT_MOB_MAJOR_COMPACTION_REGION_BATCH_SIZE);
+
+ }
+
+ @VisibleForTesting
+ public MobFileCompactionChore(Configuration conf, int batchSize) {
+this.conf = conf;
+this.regionBatchSize = batchSize;
+ }
+
+ @Override
+ protected void chore() {
+
+boolean reported = false;
+
+try (Connection conn = ConnectionFactory.createConnection(conf);
+Admin admin = conn.getAdmin();) {
+
+ TableDescriptors htds = master.getTableDescriptors();
+ Map map = htds.getAll();
+ for (TableDescriptor htd : map.values()) {
+if (!master.getTableStateManager().isTableState(htd.getTableName(),
+ TableState.State.ENABLED)) {
+ LOG.info("Skipping MOB compaction on table {} because it is not
ENABLED",
+htd.getTableName());
+ continue;
+} else {
+ LOG.info("Starting MOB compaction on table {}, checking {} column
families",
+htd.getTableName(), htd.getColumnFamilyCount());
+}
+for (ColumnFamilyDescriptor hcd : htd.getColumnFamilies()) {
+ try {
+if (hcd.isMobEnabled()) {
+ if (!reported) {
+master.reportMobCompactionStart(htd.getTableName());
+reported = true;
+ }
+ LOG.info("Major
[GitHub] [hbase] abhishek-chouhan opened a new pull request #1161: HBASE-23825 Increment proto conversion is broken
abhishek-chouhan opened a new pull request #1161: HBASE-23825 Increment proto conversion is broken URL: https://github.com/apache/hbase/pull/1161 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Commented] (HBASE-23802) Remove unnecessary Configuration instantiation in LossyAccounting
[
https://issues.apache.org/jira/browse/HBASE-23802?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17034038#comment-17034038
]
Michael Stack commented on HBASE-23802:
---
You want +1s on your backports? You went ahead w/ backports in spite of
comments above? Trying to figure how to help here.
> Remove unnecessary Configuration instantiation in LossyAccounting
> -
>
> Key: HBASE-23802
> URL: https://issues.apache.org/jira/browse/HBASE-23802
> Project: HBase
> Issue Type: Improvement
> Components: metrics
>Reporter: Nick Dimiduk
>Assignee: Nick Dimiduk
>Priority: Minor
> Fix For: 3.0.0, 2.3.0, 1.6.0
>
>
> Mighty [~stack] pointed out over on HBASE-23801 that maybe {{LossyCounting}}
> doesn't need to be instantiating a {{Configuration}} instance in the first
> place.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (HBASE-23779) Up the default fork count to make builds complete faster; make count relative to CPU count
[ https://issues.apache.org/jira/browse/HBASE-23779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033916#comment-17033916 ] Sean Busbey commented on HBASE-23779: - bq. Sean Busbey any objection if I modify the Jenkins script as follows? Sorry, some how missed this. I think the correct thing to do in the case of running a non-release is to do a build of Yetus and then consume the binary off of that. I *think* yetus precommit works directly from source, but I don't think the project promises it will nor intends folks to do that. > Up the default fork count to make builds complete faster; make count relative > to CPU count > -- > > Key: HBASE-23779 > URL: https://issues.apache.org/jira/browse/HBASE-23779 > Project: HBase > Issue Type: Bug > Components: test >Reporter: Michael Stack >Assignee: Michael Stack >Priority: Major > Fix For: 3.0.0, 2.3.0 > > Attachments: addendum2.patch, test_yetus_934.0.patch > > > Tests take a long time. Our fork count running all tests are conservative -- > 1 (small) for first part and 5 for second part (medium and large). Rather > than hardcoding we should set the fork count to be relative to machine size. > Suggestion here is 0.75C where C is CPU count. This ups the CPU use on my box. > Looking up at jenkins, it seems like the boxes are 24 cores... at least going > by my random survey. The load reported on a few seems low though this not > representative (looking at machine/uptime). > More parallelism willl probably mean more test failure. Let me take a look > see. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [hbase] saintstack commented on issue #1158: HBASE-23823 Run maven with more than default single thread (--threads…
saintstack commented on issue #1158: HBASE-23823 Run maven with more than default single thread (--threads… URL: https://github.com/apache/hbase/pull/1158#issuecomment-584331886 Patch adds to hbase-personality extras --threads=0.5C. Adds MAVEN_ARGS for when building site with --threads == 0.5C. Then it adds -XX:-MaxFDLimit as flag for surefire forked JVMs so we can up the file count for the jvm Or, oops, this seems to be a macosx special so probably won't make a difference. The build boxes are set to 16k. Lets see. 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Commented] (HBASE-23792) [Flakey Test] TestExportSnapshotNoCluster.testSnapshotWithRefsExportFileSystemState
[
https://issues.apache.org/jira/browse/HBASE-23792?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17033977#comment-17033977
]
Nick Dimiduk commented on HBASE-23792:
--
Argh.
> [Flakey Test]
> TestExportSnapshotNoCluster.testSnapshotWithRefsExportFileSystemState
> ---
>
> Key: HBASE-23792
> URL: https://issues.apache.org/jira/browse/HBASE-23792
> Project: HBase
> Issue Type: Test
> Components: test
>Affects Versions: 2.3.0
>Reporter: Nick Dimiduk
>Assignee: Nick Dimiduk
>Priority: Major
> Fix For: 3.0.0, 2.3.0, 2.1.9, 2.2.4
>
> Attachments:
> TEST-org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.xml
>
>
> {{TestExportSnapshotNoCluster.testSnapshotWithRefsExportFileSystemState}}
> fails with
> {noformat}
> java.lang.IllegalArgumentException: Wrong FS:
> file:/home/jenkins/jenkins-slave/workspace/HBase_Nightly_branch-2@2/component/hbase-mapreduce/target/test-data/878a5107-35a3-90ea-50ef-d2a3c32a50dc/.hbase-snapshot/tableWithRefsV1,
> expected: hdfs://localhost:44609
> at
> org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotWithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:110)
> at
> org.apache.hadoop.hbase.snapshot.TestExportSnapshotNoCluster.testSnapshotWithRefsExportFileSystemState(TestExportSnapshotNoCluster.java:90)
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [hbase] jatsakthi commented on issue #1138: HBASE-22827 Expose multi-region merge in shell and Admin API
jatsakthi commented on issue #1138: HBASE-22827 Expose multi-region merge in shell and Admin API URL: https://github.com/apache/hbase/pull/1138#issuecomment-584385796 @esteban , the current design supports backward compatibility of the just the 2 regions case just like how it used to be before. Adding on to it: - same syntax can be extended to provide n number of regions to merge - Also, this function would be able to take an array of regions as well, as a parameter and treat them similarly. So these commands are treated similarly: merge_region 'region_id1','region_id2' merge_region 'region_id1','region_id2','region_id3' merge_region ['region_id1','region_id2'] merge_region ['region_id1','region_id2','region_id3'] 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] busbey commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
busbey commented on a change in pull request #921: HBASE-22749: Distributed MOB
compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377358124
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
##
@@ -183,105 +271,184 @@ protected boolean performCompaction(FileDetails fd,
InternalScanner scanner, Cel
boolean hasMore;
Path path = MobUtils.getMobFamilyPath(conf, store.getTableName(),
store.getColumnFamilyName());
byte[] fileName = null;
-StoreFileWriter mobFileWriter = null, delFileWriter = null;
-long mobCells = 0, deleteMarkersCount = 0;
+StoreFileWriter mobFileWriter = null;
+/*
+ * mobCells are used only to decide if we need to commit or abort current
MOB output file.
+ */
+long mobCells = 0;
long cellsCountCompactedToMob = 0, cellsCountCompactedFromMob = 0;
long cellsSizeCompactedToMob = 0, cellsSizeCompactedFromMob = 0;
boolean finished = false;
+
ScannerContext scannerContext =
ScannerContext.newBuilder().setBatchLimit(compactionKVMax).build();
throughputController.start(compactionName);
-KeyValueScanner kvs = (scanner instanceof KeyValueScanner)?
(KeyValueScanner)scanner : null;
-long shippedCallSizeLimit = (long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+KeyValueScanner kvs = (scanner instanceof KeyValueScanner) ?
(KeyValueScanner) scanner : null;
+long shippedCallSizeLimit =
+(long) numofFilesToCompact *
this.store.getColumnFamilyDescriptor().getBlocksize();
+
+Cell mobCell = null;
try {
- try {
-// If the mob file writer could not be created, directly write the
cell to the store file.
-mobFileWriter = mobStore.createWriterInTmp(new Date(fd.latestPutTs),
fd.maxKeyCount,
- compactionCompression, store.getRegionInfo().getStartKey(), true);
-fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
- } catch (IOException e) {
-LOG.warn("Failed to create mob writer, "
- + "we will continue the compaction by writing MOB cells
directly in store files", e);
- }
- if (major) {
-try {
- delFileWriter = mobStore.createDelFileWriterInTmp(new
Date(fd.latestPutTs),
-fd.maxKeyCount, compactionCompression,
store.getRegionInfo().getStartKey());
-} catch (IOException e) {
- LOG.warn(
-"Failed to create del writer, "
-+ "we will continue the compaction by writing delete markers
directly in store files",
-e);
-}
- }
+
+ mobFileWriter = newMobWriter(fd);
+ fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
+
do {
hasMore = scanner.next(cells, scannerContext);
-if (LOG.isDebugEnabled()) {
- now = EnvironmentEdgeManager.currentTime();
-}
+now = EnvironmentEdgeManager.currentTime();
for (Cell c : cells) {
- if (major && CellUtil.isDelete(c)) {
-if (MobUtils.isMobReferenceCell(c) || delFileWriter == null) {
- // Directly write it to a store file
- writer.append(c);
+ if (compactMOBs) {
+if (MobUtils.isMobReferenceCell(c)) {
+ String fName = MobUtils.getMobFileName(c);
+ Path pp = new Path(new Path(fs.getUri()), new Path(path, fName));
+
+ // Added to support migration
+ try {
+mobCell = mobStore.resolve(c, true, false).getCell();
+ } catch (FileNotFoundException fnfe) {
+if (discardMobMiss) {
+ LOG.error("Missing MOB cell: file={} not found cell={}",
fName, c);
+ continue;
+} else {
+ throw fnfe;
+}
+ }
+
+ if (discardMobMiss && mobCell.getValueLength() == 0) {
+LOG.error("Missing MOB cell value: file={} cell={}", pp,
mobCell);
Review comment:
`mobCell` is after we resolve the backing value. We're in an error handling
path where we know something has gone wrong. We should also log the thing we
used to do the resolution, which is `c`.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377357996
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCleanerChore.java
##
@@ -0,0 +1,331 @@
+/**
+ * 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.mob;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.apache.hadoop.hbase.ScheduledChore;
+import org.apache.hadoop.hbase.TableDescriptors;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.backup.HFileArchiver;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.regionserver.BloomType;
+import org.apache.hadoop.hbase.regionserver.HStoreFile;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.hadoop.hbase.util.FSUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+
+/**
+ * The class MobFileCleanerChore for running cleaner regularly to remove the
expired
+ * and obsolete (files which have no active references to) mob files.
+ */
+@InterfaceAudience.Private
+public class MobFileCleanerChore extends ScheduledChore {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MobFileCleanerChore.class);
+ private final HMaster master;
+ private ExpiredMobFileCleaner cleaner;
+
+ static {
+Configuration.addDeprecation(MobConstants.DEPRECATED_MOB_CLEANER_PERIOD,
+ MobConstants.MOB_CLEANER_PERIOD);
+ }
+
+ public MobFileCleanerChore(HMaster master) {
+super(master.getServerName() + "-ExpiredMobFileCleanerChore", master,
+master.getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD,
+ MobConstants.DEFAULT_MOB_CLEANER_PERIOD),
+master.getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD,
+ MobConstants.DEFAULT_MOB_CLEANER_PERIOD),
+TimeUnit.SECONDS);
+this.master = master;
+cleaner = new ExpiredMobFileCleaner();
+cleaner.setConf(master.getConfiguration());
+checkObsoleteConfigurations();
+ }
+
+ private void checkObsoleteConfigurations() {
+Configuration conf = master.getConfiguration();
+
+if (conf.get("hbase.mob.compaction.mergeable.threshold") != null) {
+ LOG.warn("'hbase.mob.compaction.mergeable.threshold' is obsolete and not
used anymore.");
+}
+if (conf.get("hbase.mob.delfile.max.count") != null) {
+ LOG.warn("'hbase.mob.delfile.max.count' is obsolete and not used
anymore.");
+}
+if (conf.get("hbase.mob.compaction.threads.max") != null) {
+ LOG.warn("'hbase.mob.compaction.threads.max' is obsolete and not used
anymore.");
+}
+if (conf.get("hbase.mob.compaction.batch.size") != null) {
+ LOG.warn("'hbase.mob.compaction.batch.size' is obsolete and not used
anymore.");
+}
+ }
+
+ @VisibleForTesting
+ public MobFileCleanerChore() {
+this.master = null;
+ }
+
+ @Override
+ @edu.umd.cs.findbugs.annotations.SuppressWarnings(value =
"REC_CATCH_EXCEPTION",
+ justification = "Intentional")
+
+ protected void chore() {
+TableDescriptors htds = master.getTableDescriptors();
+
+Map map = null;
+try {
+ map = htds.getAll();
+} catch (IOException e) {
+
[GitHub] [hbase] saintstack opened a new pull request #1158: HBASE-23823 Run maven with more than default single thread (--threads…
saintstack opened a new pull request #1158: HBASE-23823 Run maven with more than default single thread (--threads… URL: https://github.com/apache/hbase/pull/1158 …=1C) 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] saintstack commented on issue #1158: HBASE-23823 Run maven with more than default single thread (--threads…
saintstack commented on issue #1158: HBASE-23823 Run maven with more than default single thread (--threads… URL: https://github.com/apache/hbase/pull/1158#issuecomment-584333726 Removed changes to the website generating script. 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Updated] (HBASE-23823) Run maven with more than default single thread (--threads=0.5C)
[ https://issues.apache.org/jira/browse/HBASE-23823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Michael Stack updated HBASE-23823: -- Summary: Run maven with more than default single thread (--threads=0.5C) (was: Run maven with more than default single thread (--threads=1C)) > Run maven with more than default single thread (--threads=0.5C) > --- > > Key: HBASE-23823 > URL: https://issues.apache.org/jira/browse/HBASE-23823 > Project: HBase > Issue Type: Sub-task >Reporter: Michael Stack >Priority: Major > > See tail of parent task where we made commits to hbase-personality trying to > make maven run more furiously on nightly builds by upping the maven thread > count (the --threads flag ... see > https://cwiki.apache.org/confluence/display/MAVEN/Parallel+builds+in+Maven+3). > The effort was abandoned because it unsettled the build. This issue > separates messing with the maven threads flag from the broader topic the > parent covers. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [hbase] bharathv commented on a change in pull request #1137: HBASE-23804: Fix default master addr hostname in master registry
bharathv commented on a change in pull request #1137: HBASE-23804: Fix default master addr hostname in master registry URL: https://github.com/apache/hbase/pull/1137#discussion_r377356634 ## File path: hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java ## @@ -181,7 +181,11 @@ /** Configuration key for the list of master host:ports **/ public static final String MASTER_ADDRS_KEY = "hbase.masters"; - public static final String MASTER_ADDRS_DEFAULT = "localhost:" + DEFAULT_MASTER_PORT; Review comment: Ack. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] bharathv merged pull request #1137: HBASE-23804: Fix default master addr hostname in master registry
bharathv merged pull request #1137: HBASE-23804: Fix default master addr hostname in master registry URL: https://github.com/apache/hbase/pull/1137 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] VladRodionov commented on a change in pull request #921: HBASE-22749: Distributed MOB compactions
VladRodionov commented on a change in pull request #921: HBASE-22749:
Distributed MOB compactions
URL: https://github.com/apache/hbase/pull/921#discussion_r377394373
##
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobFileCleanerChore.java
##
@@ -0,0 +1,331 @@
+/**
+ * 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.mob;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.apache.hadoop.hbase.ScheduledChore;
+import org.apache.hadoop.hbase.TableDescriptors;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.backup.HFileArchiver;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.regionserver.BloomType;
+import org.apache.hadoop.hbase.regionserver.HStoreFile;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.hadoop.hbase.util.FSUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import
org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+
+/**
+ * The class MobFileCleanerChore for running cleaner regularly to remove the
expired
+ * and obsolete (files which have no active references to) mob files.
+ */
+@InterfaceAudience.Private
+public class MobFileCleanerChore extends ScheduledChore {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(MobFileCleanerChore.class);
+ private final HMaster master;
+ private ExpiredMobFileCleaner cleaner;
+
+ static {
+Configuration.addDeprecation(MobConstants.DEPRECATED_MOB_CLEANER_PERIOD,
+ MobConstants.MOB_CLEANER_PERIOD);
+ }
+
+ public MobFileCleanerChore(HMaster master) {
+super(master.getServerName() + "-ExpiredMobFileCleanerChore", master,
+master.getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD,
+ MobConstants.DEFAULT_MOB_CLEANER_PERIOD),
+master.getConfiguration().getInt(MobConstants.MOB_CLEANER_PERIOD,
+ MobConstants.DEFAULT_MOB_CLEANER_PERIOD),
+TimeUnit.SECONDS);
+this.master = master;
+cleaner = new ExpiredMobFileCleaner();
+cleaner.setConf(master.getConfiguration());
+checkObsoleteConfigurations();
+ }
+
+ private void checkObsoleteConfigurations() {
+Configuration conf = master.getConfiguration();
+
+if (conf.get("hbase.mob.compaction.mergeable.threshold") != null) {
+ LOG.warn("'hbase.mob.compaction.mergeable.threshold' is obsolete and not
used anymore.");
+}
+if (conf.get("hbase.mob.delfile.max.count") != null) {
+ LOG.warn("'hbase.mob.delfile.max.count' is obsolete and not used
anymore.");
+}
+if (conf.get("hbase.mob.compaction.threads.max") != null) {
+ LOG.warn("'hbase.mob.compaction.threads.max' is obsolete and not used
anymore.");
+}
+if (conf.get("hbase.mob.compaction.batch.size") != null) {
+ LOG.warn("'hbase.mob.compaction.batch.size' is obsolete and not used
anymore.");
+}
+ }
+
+ @VisibleForTesting
+ public MobFileCleanerChore() {
+this.master = null;
+ }
+
+ @Override
+ @edu.umd.cs.findbugs.annotations.SuppressWarnings(value =
"REC_CATCH_EXCEPTION",
+ justification = "Intentional")
+
+ protected void chore() {
+TableDescriptors htds = master.getTableDescriptors();
+
+Map map = null;
+try {
+ map = htds.getAll();
+} catch (IOException e) {
+
