You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2023/01/14 00:43:05 UTC
[GitHub] [hbase] bbeaudreault opened a new pull request, #4967: HBASE-27558 Scan quotas and limits should account for total block IO
bbeaudreault opened a new pull request, #4967:
URL: https://github.com/apache/hbase/pull/4967
I still need to add tests, but I've deployed this on a test cluster and verified that it helps to reduce excess retained blocks due to heavily filtered scans. It is nice to have a upper limit on the cost of a scan, whether filtered or unfiltered and equally applying to both.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1382621790
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 38s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 57s | master passed |
| +1 :green_heart: | compile | 2m 34s | master passed |
| +1 :green_heart: | checkstyle | 0m 36s | master passed |
| +1 :green_heart: | spotless | 0m 45s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 31s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 35s | the patch passed |
| +1 :green_heart: | compile | 2m 33s | the patch passed |
| -0 :warning: | javac | 2m 33s | hbase-server generated 2 new + 193 unchanged - 2 fixed = 195 total (was 195) |
| +1 :green_heart: | checkstyle | 0m 35s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 10m 0s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 40s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 33s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | The patch does not generate ASF License warnings. |
| | | 33m 29s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux d1f70209af8a 5.4.0-1092-aws #100~18.04.2-Ubuntu SMP Tue Nov 29 08:39:52 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2a7c69d30e |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| javac | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/1/artifact/yetus-general-check/output/diff-compile-javac-hbase-server.txt |
| Max. process+thread count | 80 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/1/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1073484237
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java:
##########
@@ -612,6 +612,9 @@ public boolean next(List<Cell> outResult, ScannerContext scannerContext) throws
// here, we still need to scan all the qualifiers before returning...
scannerContext.returnImmediately();
}
+
+ scannerContext.incrementBlockProgress(heap.getCurrentBlockSizeOnce());
Review Comment:
Correct. KeyValueHeap makes this tricky, this is why the API design is strange. In the context of a single `StoreScanner.next` call, we can jump back and forth between different blocks in different StoreFiles. This approach here ensures that we always only count each block once.
I considered a few different approaches, and I felt this one was the simplest:
1. Similar to RSRpcServices.addSize, try tracking blocks here by checking `heap.getCurrentBlock() != lastBlock`. As mentioned above, this is error prone (as the comments in addSize mention as well).
2. Try adding an `HFileReaderImpl.setBlockChangedCallback(newBlock -> scannerContext.incrementBlockSize(newBlock))`. This would work, but opens up issue with how StoreScanner can re-open StoreFileScanners due to flushes or read type switches, etc. We'd need to update this callback in RegionScannerImpl.nextInternal, which would need to delegate out to all StoreScanners -> KeyValueHeaps -> StoreFileScanners.
3. Add a `HFileReaderImpl.getBlockChanged()` and `HFileReader.getCurrentBlockSize()`. Then we could change this line to:
```java
if (heap.getBlockChanged()) {
scannerContext.incrementBlockProgress(heap.getCurrentBlockSize());
}
```
The implementation of HFileReaderImpl.getBlockChanged would be:
```java
public boolean getBlockChanged() {
if (blockChanged) {
blockChanged = false;
return true;
}
return false;
}
public int getCurrentBlockSize() {
return curBlock.getUncompressedSizeWithoutHeader();
}
```
We'd reset blockChanged to true whenever we update curBlock in HFileReaderImpl.
Option 3 would probably be the closest to what I have and most accurate/simplest, and it might be more intuitive API as well. The only downside would be 2 new interface methods instead of 1.
Would you like me to do Option 3?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java:
##########
@@ -612,6 +612,9 @@ public boolean next(List<Cell> outResult, ScannerContext scannerContext) throws
// here, we still need to scan all the qualifiers before returning...
scannerContext.returnImmediately();
}
+
+ scannerContext.incrementBlockProgress(heap.getCurrentBlockSizeOnce());
Review Comment:
Correct. KeyValueHeap makes this tricky, this is why the API design is strange. In the context of a single `StoreScanner.next` call, we can jump back and forth between different blocks in different StoreFiles. This approach here ensures that we always only count each block once.
I considered a few different approaches, and I felt this one was the simplest:
1. Similar to RSRpcServices.addSize, try tracking blocks here by checking `heap.getCurrentBlock() != lastBlock`. As mentioned above, this is error prone (as the comments in addSize mention as well).
2. Try adding an `HFileReaderImpl.setBlockChangedCallback(newBlock -> scannerContext.incrementBlockSize(newBlock))`. This would work, but opens up issue with how StoreScanner can re-open StoreFileScanners due to flushes or read type switches, etc. We'd need to update this callback in RegionScannerImpl.nextInternal, which would need to delegate out to all StoreScanners -> KeyValueHeaps -> StoreFileScanners.
3. Add a `HFileReaderImpl.getBlockChanged()` and `HFileReader.getCurrentBlockSize()`. Then we could change this line to:
```java
if (heap.getBlockChanged()) {
scannerContext.incrementBlockProgress(heap.getCurrentBlockSize());
}
```
The implementation of HFileReaderImpl.getBlockChanged would be:
```java
public boolean getBlockChanged() {
if (blockChanged) {
blockChanged = false;
return true;
}
return false;
}
public int getCurrentBlockSize() {
return curBlock.getUncompressedSizeWithoutHeader();
}
```
We'd reset blockChanged to true whenever we update curBlock in HFileReaderImpl.
Option 3 would probably be the closest to what I have and most accurate/simplest, and it might be more intuitive API as well. The only downside would be 2 new interface methods instead of 1.
Would you like me to do Option 3?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache9 commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1082300876
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java:
##########
@@ -140,4 +140,10 @@ public interface HFileScanner extends Shipper, Closeable {
*/
@Override
void close();
+
+ /**
+ * Returns the block size in bytes for the current block. Will only return a value once per block,
+ * otherwise 0. Used for calculating block IO in ScannerContext.
+ */
+ int getCurrentBlockSizeOnce();
Review Comment:
I think it is better to introduce a method called recordBlockSize? The comment could say that the implementation should make sure that for every block we only record once.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3616,8 +3620,9 @@ public ScanResponse scan(final RpcController controller, final ScanRequest reque
if (region.getCoprocessorHost() != null) {
Boolean bypass = region.getCoprocessorHost().preScannerNext(scanner, results, rows);
if (!results.isEmpty()) {
+ Object lastBlock = null;
for (Result r : results) {
- lastBlock.setValue(addSize(rpcCall, r, lastBlock.getValue()));
+ lastBlock = addSize(rpcCall, r, lastBlock);
Review Comment:
I mean here, we still call addSize with lastBlock, and in addSize we have some logic for limiting.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3398,7 +3398,6 @@ private void scan(HBaseRpcController controller, ScanRequest request, RegionScan
}
boolean mayHaveMoreCellsInRow = scannerContext.mayHaveMoreCellsInRow();
Result r = Result.create(values, null, stale, mayHaveMoreCellsInRow);
- lastBlock.setValue(addSize(rpcCall, r, lastBlock.getValue()));
Review Comment:
Then could we just remove the lastBlock? Seems we still have it declared in the caller method.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java:
##########
@@ -608,6 +610,11 @@ private boolean nextInternal(List<Cell> results, ScannerContext scannerContext)
return scannerContext.setScannerState(NextState.NO_MORE_VALUES).hasMoreValues();
}
if (!shouldStop) {
+ // Read nothing as the cells were filtered, but still need to check time limit.
Review Comment:
This method is on the critial path of reading, and the code here will executed every time when we get a row, so it may affect scan performance if we add more checks here.
I just mean is it a must to have a check here? Why we do not need to check here in the past...
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1384256949
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 41s | master passed |
| +1 :green_heart: | compile | 2m 24s | master passed |
| +1 :green_heart: | checkstyle | 0m 35s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 22s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 22s | the patch passed |
| +1 :green_heart: | compile | 2m 23s | the patch passed |
| +1 :green_heart: | javac | 2m 23s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 31s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 8m 57s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 33s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | The patch does not generate ASF License warnings. |
| | | 30m 42s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux b040d47def72 5.4.0-1093-aws #102~18.04.2-Ubuntu SMP Wed Dec 7 00:31:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7ed2cb99f9 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/3/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1073472783
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3398,7 +3398,6 @@ private void scan(HBaseRpcController controller, ScanRequest request, RegionScan
}
boolean mayHaveMoreCellsInRow = scannerContext.mayHaveMoreCellsInRow();
Result r = Result.create(values, null, stale, mayHaveMoreCellsInRow);
- lastBlock.setValue(addSize(rpcCall, r, lastBlock.getValue()));
Review Comment:
This call to `addSize` here is no longer necessary for the underlying scan, because now we track blocks in the ScannerContext. This addSize call increments the `rpcCall` response cell size and block size based on the estimated block sizes. The new method is more accurate, and a few lines below we call `rpcCall.incrementResponseBlockSize` and `rpcCall.incrementResponseCellSize` to ensure the same rpc call accounting.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1089669343
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3616,8 +3620,9 @@ public ScanResponse scan(final RpcController controller, final ScanRequest reque
if (region.getCoprocessorHost() != null) {
Boolean bypass = region.getCoprocessorHost().preScannerNext(scanner, results, rows);
if (!results.isEmpty()) {
+ Object lastBlock = null;
for (Result r : results) {
- lastBlock.setValue(addSize(rpcCall, r, lastBlock.getValue()));
+ lastBlock = addSize(rpcCall, r, lastBlock);
Review Comment:
I'm still a bit confused, if we do not pass lastBlock done, how could we count the size of these blocks and limit the total size?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java:
##########
@@ -140,4 +141,11 @@ public interface HFileScanner extends Shipper, Closeable {
*/
@Override
void close();
+
+ /**
+ * Record the size of the current block in bytes, passing as an argument to the blockSizeConsumer.
+ * Implementations should ensure that blockSizeConsumer is only called once per block.
+ * @param blockSizeConsumer to be called with block size in bytes, once per block.
+ */
+ void recordBlockSize(Consumer<Integer> blockSizeConsumer);
Review Comment:
Just use IntConsumer?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1089998253
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3431,8 +3441,10 @@ private void scan(HBaseRpcController controller, ScanRequest request, RegionScan
// there are more values to be read server side. If there aren't more values,
// marking it as a heartbeat is wasteful because the client will need to issue
// another ScanRequest only to realize that they already have all the values
- if (moreRows && timeLimitReached) {
- // Heartbeat messages occur when the time limit has been reached.
+ if (moreRows && (timeLimitReached || (sizeLimitReached && results.isEmpty()))) {
Review Comment:
This code block is to send a heartbeat response. Previously sizeLimitReached was not checked here because all of the size limits could only be exceeded if results had been collected. Now with block size check, we might exceed size limit before collecting any results.
There's no need to set heartbeat response if there are results. It especially seems important not to send cursor if there are results.
Given this size limit check is new, it felt best to check results.isEmpty so we don't unnecessarily send heartbeat or cursor along with real results.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1382838915
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 41s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 23s | master passed |
| +1 :green_heart: | compile | 0m 34s | master passed |
| +1 :green_heart: | shadedjars | 4m 0s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 7s | the patch passed |
| +1 :green_heart: | compile | 0m 35s | the patch passed |
| +1 :green_heart: | javac | 0m 35s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 0s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 208m 5s | hbase-server in the patch passed. |
| | | 226m 58s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 066ee123b86d 5.4.0-1092-aws #100~18.04.2-Ubuntu SMP Tue Nov 29 08:39:52 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2a7c69d30e |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/2/testReport/ |
| Max. process+thread count | 2489 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/2/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1398897337
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 41s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 11s | master passed |
| +1 :green_heart: | compile | 0m 34s | master passed |
| +1 :green_heart: | shadedjars | 4m 30s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 41s | the patch passed |
| +1 :green_heart: | compile | 0m 34s | the patch passed |
| +1 :green_heart: | javac | 0m 34s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 32s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 20s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 208m 1s | hbase-server in the patch passed. |
| | | 229m 4s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 94e5cced639e 5.4.0-1092-aws #100~18.04.2-Ubuntu SMP Tue Nov 29 08:39:52 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 913cf6b96d |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/4/testReport/ |
| Max. process+thread count | 3038 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1382732378
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 38s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 33s | master passed |
| +1 :green_heart: | compile | 2m 31s | master passed |
| +1 :green_heart: | checkstyle | 0m 34s | master passed |
| +1 :green_heart: | spotless | 0m 41s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 27s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 32s | the patch passed |
| +1 :green_heart: | compile | 2m 31s | the patch passed |
| -0 :warning: | javac | 2m 31s | hbase-server generated 2 new + 193 unchanged - 2 fixed = 195 total (was 195) |
| +1 :green_heart: | checkstyle | 0m 32s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 9m 51s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 41s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 8s | The patch does not generate ASF License warnings. |
| | | 32m 29s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux d2229de451d4 5.4.0-1092-aws #100~18.04.2-Ubuntu SMP Tue Nov 29 08:39:52 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2a7c69d30e |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| javac | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/2/artifact/yetus-general-check/output/diff-compile-javac-hbase-server.txt |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/2/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1082800609
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java:
##########
@@ -608,6 +610,11 @@ private boolean nextInternal(List<Cell> results, ScannerContext scannerContext)
return scannerContext.setScannerState(NextState.NO_MORE_VALUES).hasMoreValues();
}
if (!shouldStop) {
+ // Read nothing as the cells were filtered, but still need to check time limit.
Review Comment:
Actually, this section of the method is not nearly as hot as the rest of the method. The only real way we reach this point is when `filter.filterRowCells(kvs)` clears all cells from the results after having been accumulated in StoreScanner. There are only 2 standard filters which do this -- DependentColumnFilter and SingleColumnValueExcludeFilter.
That said, you do make a good point. We have never had a time limit here, so we may not need it. We _do_ need a size limit check here, now that we track block sizes. Previously, we would not check size limit here because the results are empty so wouldn't have accumulated size progress. Now that we accumulate block size progress even for filtered rows, we need a check.
For this type of scan, we will have accumulated blocks in both `populateResults()` and possibly `nextRow()`. Right after `populateResults()` there's a `scannerContext.checkAnyLimitReached(LimitScope.BETWEEN_CELLS)` call. That call doesn't protect against this case, because it passes `BETWEEN_CELLS`. For scans with `filter.hasFilterRow()`, the limit scope is changed to `LimitScope.BETWEEN_ROWS`. So this check is skipped for these. Scans which enter this code block will have skipped all other limit checks above. The checkSizeLimit I add here is the only safe place we can check BETWEEN_ROWS for these types of filtered scans.
The best way to illustrate this is with a test -- I just pushed a change which does the following:
1. Change this line to just `checkSizeLimit`
2. Adds a new test `testCheckLimitAfterFilteringRowCells`
If I comment out this checkSizeLimit, the added test fails -- the whole scan is able to complete in 1 rpc instead of the expected 4. So this illustrates that we need to have a size check here.
Personally I think it's also accurate to have a time limit check here, because for these types of scans I think they'd be able to circumvent our existing time limits. But within the scope of this JIRA, I can keep it to just size limit for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1082802553
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3398,7 +3398,6 @@ private void scan(HBaseRpcController controller, ScanRequest request, RegionScan
}
boolean mayHaveMoreCellsInRow = scannerContext.mayHaveMoreCellsInRow();
Result r = Result.create(values, null, stale, mayHaveMoreCellsInRow);
- lastBlock.setValue(addSize(rpcCall, r, lastBlock.getValue()));
Review Comment:
I removed `lastBlock` from this method, but we still need it in the caller. I explain the reasonings in my response to your other comment.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1089955244
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java:
##########
@@ -336,6 +337,9 @@ protected static class HFileScannerImpl implements HFileScanner {
// RegionScannerImpl#handleException). Call the releaseIfNotCurBlock() to release the
// unreferenced block please.
protected HFileBlock curBlock;
+ // Whether we returned a result for curBlock's size in getCurrentBlockSizeOnce().
Review Comment:
getCurrentBlockSizeOnce() -> recordBlockSize()
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3431,8 +3441,10 @@ private void scan(HBaseRpcController controller, ScanRequest request, RegionScan
// there are more values to be read server side. If there aren't more values,
// marking it as a heartbeat is wasteful because the client will need to issue
// another ScanRequest only to realize that they already have all the values
- if (moreRows && timeLimitReached) {
- // Heartbeat messages occur when the time limit has been reached.
+ if (moreRows && (timeLimitReached || (sizeLimitReached && results.isEmpty()))) {
Review Comment:
Why sizeLimitReached needs an extra results.isEmpty check?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3343,7 +3341,20 @@ private void scan(HBaseRpcController controller, ScanRequest request, RegionScan
ScannerContext.Builder contextBuilder = ScannerContext.newBuilder(true);
// maxResultSize - either we can reach this much size for all cells(being read) data or sum
// of heap size occupied by cells(being read). Cell data means its key and value parts.
- contextBuilder.setSizeLimit(sizeScope, maxResultSize, maxResultSize);
+ // maxQuotaResultSize - max results just from server side configuration and quotas, without
+
Review Comment:
Why an empty line here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1398701858
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 1s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 21s | master passed |
| +1 :green_heart: | compile | 2m 25s | master passed |
| +1 :green_heart: | checkstyle | 0m 38s | master passed |
| +1 :green_heart: | spotless | 0m 43s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 29s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 12s | the patch passed |
| +1 :green_heart: | compile | 2m 21s | the patch passed |
| +1 :green_heart: | javac | 2m 21s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 36s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 13m 16s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 52s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 55s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | The patch does not generate ASF License warnings. |
| | | 40m 3s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 38e1b6a7f317 5.4.0-131-generic #147-Ubuntu SMP Fri Oct 14 17:07:22 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 913cf6b96d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/4/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1398890375
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 41s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 54s | master passed |
| +1 :green_heart: | compile | 0m 41s | master passed |
| +1 :green_heart: | shadedjars | 4m 32s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 11s | the patch passed |
| +1 :green_heart: | compile | 0m 41s | the patch passed |
| +1 :green_heart: | javac | 0m 41s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 31s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 198m 38s | hbase-server in the patch passed. |
| | | 221m 12s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 49b5075ac9b2 5.4.0-1088-aws #96~18.04.1-Ubuntu SMP Mon Oct 17 02:57:48 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 913cf6b96d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/4/testReport/ |
| Max. process+thread count | 2884 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache9 commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1073268854
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java:
##########
@@ -140,4 +140,10 @@ public interface HFileScanner extends Shipper, Closeable {
*/
@Override
void close();
+
+ /**
+ * Returns the block size in bytes for the current block. Will only return a value once per block,
+ * otherwise 0. Used for calculating block IO in ScannerContext.
+ */
+ int getCurrentBlockSizeOnce();
Review Comment:
This API design is a bit strange... Let me take a look on the usage...
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScannerContext.java:
##########
@@ -158,10 +167,32 @@ void setKeepProgress(boolean keepProgress) {
this.keepProgress = keepProgress;
}
+ /**
+ * In this mode, only block size progress is tracked, and limits are ignored. We set this mode
+ * when skipping to next row, in which case all cells returned a thrown away so should not count
Review Comment:
all cells returned [a] thrown -> all cells returned [are] thrown away?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java:
##########
@@ -608,6 +610,11 @@ private boolean nextInternal(List<Cell> results, ScannerContext scannerContext)
return scannerContext.setScannerState(NextState.NO_MORE_VALUES).hasMoreValues();
}
if (!shouldStop) {
+ // Read nothing as the cells were filtered, but still need to check time limit.
Review Comment:
Why we need to add a check here?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java:
##########
@@ -705,13 +712,21 @@ public int size() {
protected boolean nextRow(ScannerContext scannerContext, Cell curRowCell) throws IOException {
assert this.joinedContinuationRow == null : "Trying to go to next row during joinedHeap read.";
+
+ // Enable skipping row mode, which disables limits and skips tracking progress for all
+ // but block size. We keep tracking block size because skipping a row in this way
+ // might involve reading blocks along the way.
+ scannerContext.setSkippingRow(true);
+
Cell next;
while ((next = this.storeHeap.peek()) != null && CellUtil.matchingRows(next, curRowCell)) {
// Check for thread interrupt status in case we have been signaled from
// #interruptRegionOperation.
region.checkInterrupt();
- this.storeHeap.next(MOCKED_LIST);
+ this.storeHeap.next(MOCKED_LIST, scannerContext);
Review Comment:
So this is why we need the skippingRow flag? In the past we do not passing ScannerContext in, and now we want to record the block size so we need to pass it down, but we do not want to increase the size other than block?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3398,7 +3398,6 @@ private void scan(HBaseRpcController controller, ScanRequest request, RegionScan
}
boolean mayHaveMoreCellsInRow = scannerContext.mayHaveMoreCellsInRow();
Result r = Result.create(values, null, stale, mayHaveMoreCellsInRow);
- lastBlock.setValue(addSize(rpcCall, r, lastBlock.getValue()));
Review Comment:
We do not need to count lastBlock any more?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java:
##########
@@ -612,6 +612,9 @@ public boolean next(List<Cell> outResult, ScannerContext scannerContext) throws
// here, we still need to scan all the qualifiers before returning...
scannerContext.returnImmediately();
}
+
+ scannerContext.incrementBlockProgress(heap.getCurrentBlockSizeOnce());
Review Comment:
So the problem here is we do not know if this is a new block?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1082801270
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java:
##########
@@ -140,4 +140,10 @@ public interface HFileScanner extends Shipper, Closeable {
*/
@Override
void close();
+
+ /**
+ * Returns the block size in bytes for the current block. Will only return a value once per block,
+ * otherwise 0. Used for calculating block IO in ScannerContext.
+ */
+ int getCurrentBlockSizeOnce();
Review Comment:
That seems intuitive and reasonable. Done. Please see latest 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1382672911
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 7m 7s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 45s | master passed |
| +1 :green_heart: | compile | 1m 6s | master passed |
| +1 :green_heart: | shadedjars | 4m 23s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 27s | the patch passed |
| +1 :green_heart: | compile | 0m 59s | the patch passed |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 18s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 300m 2s | hbase-server in the patch failed. |
| | | 331m 21s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux aeba07a71517 5.4.0-131-generic #147-Ubuntu SMP Fri Oct 14 17:07:22 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2a7c69d30e |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/1/testReport/ |
| Max. process+thread count | 2524 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1089687483
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileScanner.java:
##########
@@ -140,4 +141,11 @@ public interface HFileScanner extends Shipper, Closeable {
*/
@Override
void close();
+
+ /**
+ * Record the size of the current block in bytes, passing as an argument to the blockSizeConsumer.
+ * Implementations should ensure that blockSizeConsumer is only called once per block.
+ * @param blockSizeConsumer to be called with block size in bytes, once per block.
+ */
+ void recordBlockSize(Consumer<Integer> blockSizeConsumer);
Review Comment:
Will fix
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1384465139
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 41s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 3s | master passed |
| +1 :green_heart: | compile | 0m 34s | master passed |
| +1 :green_heart: | shadedjars | 3m 59s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 5s | the patch passed |
| +1 :green_heart: | compile | 0m 35s | the patch passed |
| +1 :green_heart: | javac | 0m 35s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 58s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 207m 57s | hbase-server in the patch passed. |
| | | 226m 24s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 281000a9a413 5.4.0-1092-aws #100~18.04.2-Ubuntu SMP Tue Nov 29 08:39:52 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7ed2cb99f9 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/3/testReport/ |
| Max. process+thread count | 2494 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/3/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1384212079
Pushed tests, marking ready for review.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1384458987
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 41s | master passed |
| +1 :green_heart: | compile | 0m 41s | master passed |
| +1 :green_heart: | shadedjars | 3m 57s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 28s | the patch passed |
| +1 :green_heart: | compile | 0m 41s | the patch passed |
| +1 :green_heart: | javac | 0m 41s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 53s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 198m 59s | hbase-server in the patch passed. |
| | | 219m 4s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0f583f02d739 5.4.0-1088-aws #96~18.04.1-Ubuntu SMP Mon Oct 17 02:57:48 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7ed2cb99f9 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/3/testReport/ |
| Max. process+thread count | 2673 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/3/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1407714451
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 14s | master passed |
| +1 :green_heart: | compile | 2m 25s | master passed |
| +1 :green_heart: | checkstyle | 0m 33s | master passed |
| +1 :green_heart: | spotless | 0m 39s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 13s | the patch passed |
| +1 :green_heart: | compile | 2m 29s | the patch passed |
| +1 :green_heart: | javac | 2m 29s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 33s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 47s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 32s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 8s | The patch does not generate ASF License warnings. |
| | | 37m 46s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 4ee219d0bebd 5.4.0-1093-aws #102~18.04.2-Ubuntu SMP Wed Dec 7 00:31:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 382681e2d6 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/6/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1089688575
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3616,8 +3620,9 @@ public ScanResponse scan(final RpcController controller, final ScanRequest reque
if (region.getCoprocessorHost() != null) {
Boolean bypass = region.getCoprocessorHost().preScannerNext(scanner, results, rows);
if (!results.isEmpty()) {
+ Object lastBlock = null;
for (Result r : results) {
- lastBlock.setValue(addSize(rpcCall, r, lastBlock.getValue()));
+ lastBlock = addSize(rpcCall, r, lastBlock);
Review Comment:
Ok I see what you're saying. Yes these blocks are not counted towards the max size. Just counted for reporting metrics and slow log threshold.
These are two separate data sets, but I can link them. When creating the maxQuotaResultSize, I can subtract rpcCall.getResponseBlockSize(). That will account for the blocks accumulated in the coprocessor.
I won't need lastBlock. We can't really use that below now that we use scanner context which is far more accurate than addSize estimation. We could theoretically try piping it through all the way into HFileReaderImpl where we record block sizes. But I don't think it's worth it for one block overlap.
I will make the change to subtract counter block size when setting limits on ScannerContext.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1073488012
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java:
##########
@@ -608,6 +610,11 @@ private boolean nextInternal(List<Cell> results, ScannerContext scannerContext)
return scannerContext.setScannerState(NextState.NO_MORE_VALUES).hasMoreValues();
}
if (!shouldStop) {
+ // Read nothing as the cells were filtered, but still need to check time limit.
Review Comment:
We want to check the size limit any time we are potentially continuing the loop. This was the only case we missed, the others (which i converted from checkTimeLimit to checkAnyLimit above) are all similar.
Since `nextRow` is now accumulating block size, we want to check after calling nextRow to ensure we haven't exceeded the limit.
I could make this checkSizeLimit if you'd like. I made it checkAnyLimitReached so that it is the same as the other calls above, which were just checking time 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1073566824
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java:
##########
@@ -612,6 +612,9 @@ public boolean next(List<Cell> outResult, ScannerContext scannerContext) throws
// here, we still need to scan all the qualifiers before returning...
scannerContext.returnImmediately();
}
+
+ scannerContext.incrementBlockProgress(heap.getCurrentBlockSizeOnce());
Review Comment:
@Apache9 here's a commit which implements the `getBlockChanged()` approach: https://github.com/HubSpot/hbase/commit/c619b4f3f7cd7f9e9677b265e10f0f10edefcf23
Let me know if you'd like me to push that here instead. It's better in some ways, but still has one unfortunate caveat described in the javadoc of KeyValueScanner. See the "Note:" portion here:
```java
/**
* Returns true if the current block has changed since last invocation. Subsequent calls to this
* method will return false, until the block changes again. <br>
* Note: This value is propagated up from the HFile.Reader. The value of this result is only valid
* until the next call to {@link #next()} or any other seek-related method. Calling those methods
* may change which underlying HFile is currently being read, which will reset the block changed
* state.
*/
```
This is ok for our current use-case, since we always invoke the 2 methods together. But in some ways it would be better to combine these 2 methods into one (like in `getCurrentBlockSizeOnce()`) so that you can't stumble across that note.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1407756240
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 40s | master passed |
| +1 :green_heart: | compile | 0m 34s | master passed |
| +1 :green_heart: | shadedjars | 4m 33s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 49s | the patch passed |
| +1 :green_heart: | compile | 0m 35s | the patch passed |
| +1 :green_heart: | javac | 0m 35s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 34s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 203m 17s | hbase-server in the patch passed. |
| | | 225m 7s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d2d1274db8ba 5.4.0-1092-aws #100~18.04.2-Ubuntu SMP Tue Nov 29 08:39:52 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 382681e2d6 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/6/testReport/ |
| Max. process+thread count | 2446 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/6/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1407580802
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 4m 36s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 5s | master passed |
| +1 :green_heart: | compile | 0m 48s | master passed |
| +1 :green_heart: | shadedjars | 4m 23s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 17s | the patch passed |
| +1 :green_heart: | compile | 0m 48s | the patch passed |
| +1 :green_heart: | javac | 0m 48s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 22s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 209m 42s | hbase-server in the patch passed. |
| | | 236m 53s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c084488c4493 5.4.0-137-generic #154-Ubuntu SMP Thu Jan 5 17:03:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 45fd3f628a |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/5/testReport/ |
| Max. process+thread count | 2413 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/5/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault merged pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault merged PR #4967:
URL: https://github.com/apache/hbase/pull/4967
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1073473607
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java:
##########
@@ -705,13 +712,21 @@ public int size() {
protected boolean nextRow(ScannerContext scannerContext, Cell curRowCell) throws IOException {
assert this.joinedContinuationRow == null : "Trying to go to next row during joinedHeap read.";
+
+ // Enable skipping row mode, which disables limits and skips tracking progress for all
+ // but block size. We keep tracking block size because skipping a row in this way
+ // might involve reading blocks along the way.
+ scannerContext.setSkippingRow(true);
+
Cell next;
while ((next = this.storeHeap.peek()) != null && CellUtil.matchingRows(next, curRowCell)) {
// Check for thread interrupt status in case we have been signaled from
// #interruptRegionOperation.
region.checkInterrupt();
- this.storeHeap.next(MOCKED_LIST);
+ this.storeHeap.next(MOCKED_LIST, scannerContext);
Review Comment:
Yes, we used to pass in NoLimitsScannerContext here as the default. But now we want to track block size. So the `setSkippingRow` state emulates NoLimitsScannerContext.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1082509664
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3616,8 +3620,9 @@ public ScanResponse scan(final RpcController controller, final ScanRequest reque
if (region.getCoprocessorHost() != null) {
Boolean bypass = region.getCoprocessorHost().preScannerNext(scanner, results, rows);
if (!results.isEmpty()) {
+ Object lastBlock = null;
for (Result r : results) {
- lastBlock.setValue(addSize(rpcCall, r, lastBlock.getValue()));
+ lastBlock = addSize(rpcCall, r, lastBlock);
Review Comment:
Right, so these `results` here come from a coprocessor. We still want to count the size of those, and the coprocessor does not have access to the scanner context.
The way addSize works is it keeps track of the last block and only increments if `block != lastBlock`. This approach is ok for the coprocessor results.
Previously the `lastBlock` value was kept and then passed into the downstream `scan(..)` method in the MutableObject. Then where you commented before (line 3401) we would call addSize again, and the passed in `lastBlock` value would be used for that equality check.
Now that we use ScannerContext for the accounting of the actual scans, we no longer need the addSize call on line 3401. So I removed that and the MutableObject.
But we still need lastBlock for checking the coprocessor results, since the way addSize works is to keep track of a lastBlock across invocations. `addSize` is also still used by Get/Multiget (though I hope to unify those in a later issue).
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1407754660
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 47s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 35s | master passed |
| +1 :green_heart: | compile | 0m 40s | master passed |
| +1 :green_heart: | shadedjars | 4m 34s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 11s | the patch passed |
| +1 :green_heart: | compile | 0m 42s | the patch passed |
| +1 :green_heart: | javac | 0m 42s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 33s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 194m 18s | hbase-server in the patch passed. |
| | | 216m 55s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 3d98b9116176 5.4.0-1094-aws #102~18.04.1-Ubuntu SMP Tue Jan 10 21:07:03 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 382681e2d6 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/6/testReport/ |
| Max. process+thread count | 2706 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/6/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1089999190
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3431,8 +3441,10 @@ private void scan(HBaseRpcController controller, ScanRequest request, RegionScan
// there are more values to be read server side. If there aren't more values,
// marking it as a heartbeat is wasteful because the client will need to issue
// another ScanRequest only to realize that they already have all the values
- if (moreRows && timeLimitReached) {
- // Heartbeat messages occur when the time limit has been reached.
+ if (moreRows && (timeLimitReached || (sizeLimitReached && results.isEmpty()))) {
Review Comment:
Clarified in code
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1090065051
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3431,8 +3441,10 @@ private void scan(HBaseRpcController controller, ScanRequest request, RegionScan
// there are more values to be read server side. If there aren't more values,
// marking it as a heartbeat is wasteful because the client will need to issue
// another ScanRequest only to realize that they already have all the values
- if (moreRows && timeLimitReached) {
- // Heartbeat messages occur when the time limit has been reached.
+ if (moreRows && (timeLimitReached || (sizeLimitReached && results.isEmpty()))) {
Review Comment:
I tried removing the `results.isEmpty()` part here and it causes TestRawAsyncScanCursor.testSizeLimit to fail due to an unexpected cursor. I tried testing on this branch, and on branch-2 (without this patch). So I think it's good to keep `results.isEmpty()` so that we only return cursor when expected.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1089854145
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -3616,8 +3620,9 @@ public ScanResponse scan(final RpcController controller, final ScanRequest reque
if (region.getCoprocessorHost() != null) {
Boolean bypass = region.getCoprocessorHost().preScannerNext(scanner, results, rows);
if (!results.isEmpty()) {
+ Object lastBlock = null;
for (Result r : results) {
- lastBlock.setValue(addSize(rpcCall, r, lastBlock.getValue()));
+ lastBlock = addSize(rpcCall, r, lastBlock);
Review Comment:
I just pushed fixes, and added comments about what's happening with addSize and ScannerContext
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1073484237
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java:
##########
@@ -612,6 +612,9 @@ public boolean next(List<Cell> outResult, ScannerContext scannerContext) throws
// here, we still need to scan all the qualifiers before returning...
scannerContext.returnImmediately();
}
+
+ scannerContext.incrementBlockProgress(heap.getCurrentBlockSizeOnce());
Review Comment:
Correct. KeyValueHeap makes this tricky, this is why the API design is strange. In the context of a single `StoreScanner.next` call, we can jump back and forth between different blocks in different StoreFiles. This approach here ensures that we always only count each block once.
I considered a few different approaches, and I felt this one was the simplest:
1. Similar to RSRpcServices.addSize, try tracking blocks here by checking `heap.getCurrentBlock() != lastBlock`. As mentioned above, this is error prone (as the comments in addSize mention as well).
2. Try adding an `HFileReaderImpl.setBlockChangedCallback(newBlock -> scannerContext.incrementBlockSize(newBlock))`. This would work, but opens up issue with how StoreScanner can re-open StoreFileScanners due to flushes or read type switches, etc. We'd need to update this callback in RegionScannerImpl.nextInternal, which would need to delegate out to all StoreScanners -> KeyValueHeaps -> StoreFileScanners.
3. Add a `HFileReaderImpl.getBlockChanged()` and `HFileReader.getCurrentBlockSize()`. Then we could change this line to:
```java
if (heap.getBlockChanged()) {
scannerContext.incrementBlockProgress(heap.getCurrentBlockSize());
}
```
The implementation of HFileReaderImpl.getBlockChanged would be:
```java
public boolean getBlockChanged() {
if (blockChanged) {
blockChanged = false;
return true;
}
return false;
}
```
We'd reset blockChanged to true whenever we update curBlock in HFileReaderImpl.
Option 3 would probably be the closest to what I have and most accurate/simplest, and it might be more intuitive API as well. The only downside would be 2 new interface methods instead of 1.
Would you like me to do Option 3?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4967:
URL: https://github.com/apache/hbase/pull/4967#discussion_r1073489990
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java:
##########
@@ -608,6 +610,11 @@ private boolean nextInternal(List<Cell> results, ScannerContext scannerContext)
return scannerContext.setScannerState(NextState.NO_MORE_VALUES).hasMoreValues();
}
if (!shouldStop) {
+ // Read nothing as the cells were filtered, but still need to check time limit.
Review Comment:
To be honest, I think this should have been here all along and was just missed along the way. I'm not sure why we'd want to check time limit for the nextRow calls above but not this one. This check here ensures that populating from joined heap + nextRow does not exceed time or (new) size 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1398657200
Thanks for sticking with me on this @Apache9. Great feedback. I've pushed changes based on your recent comments and provided some extra context on a couple of your comments. For some reason they don't show up as responses to your most recent review, but you should see them if you scroll up.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1407587638
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 40s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 5s | master passed |
| +1 :green_heart: | compile | 0m 34s | master passed |
| +1 :green_heart: | shadedjars | 4m 31s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 39s | the patch passed |
| +1 :green_heart: | compile | 0m 34s | the patch passed |
| +1 :green_heart: | javac | 0m 34s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 31s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 253m 32s | hbase-server in the patch failed. |
| | | 275m 11s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux aedc7881a740 5.4.0-1092-aws #100~18.04.2-Ubuntu SMP Tue Nov 29 08:39:52 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 45fd3f628a |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/5/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/5/testReport/ |
| Max. process+thread count | 2477 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/5/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1407551482
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 30s | master passed |
| +1 :green_heart: | compile | 2m 23s | master passed |
| +1 :green_heart: | checkstyle | 0m 33s | master passed |
| +1 :green_heart: | spotless | 0m 39s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 5s | the patch passed |
| +1 :green_heart: | compile | 2m 19s | the patch passed |
| +1 :green_heart: | javac | 2m 19s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 33s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 17s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 24s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 9s | The patch does not generate ASF License warnings. |
| | | 36m 36s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 46fdea805df4 5.4.0-1093-aws #102~18.04.2-Ubuntu SMP Wed Dec 7 00:31:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 45fd3f628a |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 78 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/5/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4967: HBASE-27558 Scan quotas and limits should account for total block IO
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4967:
URL: https://github.com/apache/hbase/pull/4967#issuecomment-1382846123
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 59s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 3s | master passed |
| +1 :green_heart: | compile | 0m 49s | master passed |
| +1 :green_heart: | shadedjars | 3m 56s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 28s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 43s | the patch passed |
| +1 :green_heart: | compile | 0m 49s | the patch passed |
| +1 :green_heart: | javac | 0m 49s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 55s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 222m 52s | hbase-server in the patch failed. |
| | | 243m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4967 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 56b92b2bfd7d 5.4.0-131-generic #147-Ubuntu SMP Fri Oct 14 17:07:22 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2a7c69d30e |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/2/testReport/ |
| Max. process+thread count | 2545 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4967/2/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org