You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "bbeaudreault (via GitHub)" <gi...@apache.org> on 2023/03/14 01:53:16 UTC
[GitHub] [hbase] bbeaudreault opened a new pull request, #5099: HBASE-27704 Quotas can drastically overflow configured limit
bbeaudreault opened a new pull request, #5099:
URL: https://github.com/apache/hbase/pull/5099
Removes limits in `refill` which did not allow to go negative.
FixedIntervalRateLimiter also needed some changes in getWaitInterval to properly match with how refilling really works.
Updated the tests to revive the original intended logic for quotas, where we really test for overconsumption.
--
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 #5099: HBASE-27704 Quotas can drastically overflow configured limit
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5099:
URL: https://github.com/apache/hbase/pull/5099#issuecomment-1467262179
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 24s | 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 42s | master passed |
| +1 :green_heart: | compile | 2m 24s | master passed |
| +1 :green_heart: | checkstyle | 0m 34s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 13s | the patch passed |
| +1 :green_heart: | compile | 2m 20s | the patch passed |
| +1 :green_heart: | javac | 2m 20s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 30s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 46s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 37s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 27s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | The patch does not generate ASF License warnings. |
| | | 37m 44s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5099/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5099 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 836966f77f32 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 / c2b64e7caa |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 82 (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-5099/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] ndimiduk commented on pull request #5099: HBASE-27704 Quotas can drastically overflow configured limit
Posted by "ndimiduk (via GitHub)" <gi...@apache.org>.
ndimiduk commented on PR #5099:
URL: https://github.com/apache/hbase/pull/5099#issuecomment-1485103469
Is there an off-the-shelf rate limiter that we can use instead of our own implementation?
--
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 #5099: HBASE-27704 Quotas can drastically overflow configured limit
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5099:
URL: https://github.com/apache/hbase/pull/5099#issuecomment-1467402076
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 23s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 41s | master passed |
| +1 :green_heart: | compile | 0m 40s | master passed |
| +1 :green_heart: | shadedjars | 4m 27s | 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 | 3m 13s | 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 28s | 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 | 199m 39s | hbase-server in the patch passed. |
| | | 222m 25s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5099/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5099 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 5b2431fe935f 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 / c2b64e7caa |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5099/1/testReport/ |
| Max. process+thread count | 2437 (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-5099/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 pull request #5099: HBASE-27704 Quotas can drastically overflow configured limit
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5099:
URL: https://github.com/apache/hbase/pull/5099#issuecomment-1479423790
@Apache9 Thanks for taking a look. I agree the API is weird.
I think the main reason is, unlike a normal RateLimiter, in HBase we can't simply do `rateLimiter.acquire(permits)`. When a request comes in (Get/Scan), we don't know how many bytes it will consume. We also have a bunch of RateLimiters of various types to check.
So at a higher level, we do `quota.checkQuota` which does two loops of all rate limiters with an estimated read size (100 bytes for get, 1000 bytes for scan):
- The first loop checks `canExecute` of the estimate bytes, which refills and confirms whether after refill there are enough.
- The second loop does `consume` of the estimate, which actually acquires them from the rate limiter.
Once the request finishes, we know how much bytes it consumed. At that point we call `consume` once more, with the real byte value minus the estimate. This can overflow the rate limiter.
I was not around for the initial implementation of this. I tried looking up reviews from back then, but we weren't on GitHub yet so it's only in reviewboard where the review was minimal.
My guess on why they went with 2 loops is to minimize the chance that the 5th rate limiter (or whatever) fails to consume, and then you have to rollback all the ones that succeeded in consuming.
> Why we do a 'Long.MAX_VALUE + amount'? It will always overflow?
In that code snippet, `amount` is negative. So despite being confusing to read, it won't overflow. I agree its confusing though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #5099: HBASE-27704 Quotas can drastically overflow configured limit
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5099:
URL: https://github.com/apache/hbase/pull/5099#issuecomment-1474951767
@infraio any chance you might have time to take a look at this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #5099: HBASE-27704 Quotas can drastically overflow configured limit
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5099:
URL: https://github.com/apache/hbase/pull/5099#issuecomment-1467415155
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 51s | 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 57s | master passed |
| +1 :green_heart: | compile | 0m 39s | master passed |
| +1 :green_heart: | shadedjars | 4m 14s | 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 | 2m 46s | the patch passed |
| +1 :green_heart: | compile | 0m 39s | the patch passed |
| +1 :green_heart: | javac | 0m 39s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 15s | 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 | 218m 30s | hbase-server in the patch passed. |
| | | 240m 14s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5099/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5099 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux b3043fee6145 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 / c2b64e7caa |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5099/1/testReport/ |
| Max. process+thread count | 2411 (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-5099/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] Apache9 commented on pull request #5099: HBASE-27704 Quotas can drastically overflow configured limit
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5099:
URL: https://github.com/apache/hbase/pull/5099#issuecomment-1479245014
The implementation of RateLimiter is a bit strange to me. The name of the 'canExecute' method indicates that it just tests whether a request can execute, which should be read only, but in the method we will update avail? Then what is the purpose for consume method?
And in consume method
```
public synchronized void consume(final long amount) {
if (isBypass()) {
return;
}
if (amount >= 0) {
this.avail -= amount;
if (this.avail < 0) {
this.avail = 0;
}
} else {
if (this.avail <= Long.MAX_VALUE + amount) { // <===== here
this.avail -= amount;
this.avail = Math.min(this.avail, this.limit);
} else {
this.avail = this.limit;
}
}
}
```
Why we do a 'Long.MAX_VALUE + amount'? It will always overflow?
--
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 #5099: HBASE-27704 Quotas can drastically overflow configured limit
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5099:
URL: https://github.com/apache/hbase/pull/5099#issuecomment-1485219620
Thanks again nick! I agree it'd be nice if there were an OTS rate limiter we can use. That was my first thought here as well, since it took me a bit to really grok what was happening here.
The most obvious candidate is guava, but unfortunately it doesn't really work with how we currently use rate limiters in quotas. As described above, we need sort of a multi-phase commit rate limiter where you can lease and return permits, check for permits without consuming, and consume non-blocking (after request is finished, when we finally know how much we needed).
For this issue I'm partial to merging as is. I can give a quick search around for rate limiters like this and file a jira if any look promising.
--
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 #5099: HBASE-27704 Quotas can drastically overflow configured limit
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault merged PR #5099:
URL: https://github.com/apache/hbase/pull/5099
--
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