Dimitri BEKALE created HBASE-30485:
--------------------------------------
Summary: ExceedOperationQuota passes writeCapacityUnitConsumed as
the readCapacityUnit argument of grabQuota
Key: HBASE-30485
URL: https://issues.apache.org/jira/browse/HBASE-30485
Project: HBase
Issue Type: Bug
Components: Quotas
Affects Versions: 2.6.6, 3.0.0-beta-2
Reporter: Dimitri BEKALE
When exceed-throttle is enabled (\{{Admin.exceedThrottleQuotaSwitch(true)}}),
{\{RegionServerRpcQuotaManager}} uses \{{ExceedOperationQuota}}. Its
\{{checkQuota}} method debits
the limiters with \{{grabQuota}}, and both calls pass the write capacity-unit
estimate in the read
capacity-unit position
([ExceedOperationQuota.java:97-104|https://github.com/apache/hbase/blob/09474ab77bfa8bede96131164fd4451f093a1a10/hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/ExceedOperationQuota.java#L97-L104]):
{code:java}
regionServerLimiter.grabQuota(numWrites, writeConsumed, numReads + numScans,
readConsumed,
writeCapacityUnitConsumed, writeCapacityUnitConsumed, isAtomic,
handlerUsageTimeConsumed);
...
limiter.grabQuota(numWrites, writeConsumed, numReads + numScans, readConsumed,
writeCapacityUnitConsumed, writeCapacityUnitConsumed, isAtomic, 0L);
{code}
The signature is {{grabQuota(writeReqs, writeSize, readReqs, readSize,
writeCapacityUnit,
readCapacityUnit, isAtomic, estimateHandlerThreadUsageMs)}}
(\{{QuotaLimiter.java:70}}): the fifth
and sixth arguments are the write and read capacity units. The
other three call sites in the same class hierarchy pass
\{{readCapacityUnitConsumed}} there:
{\{ExceedOperationQuota.checkQuota}} step 2 (line 82),
\{{DefaultOperationQuota.checkQuota}} (line
151), and \{{ExceedOperationQuota.close()}} via \{{consumeRead}}.
h3. Effect
{\{TimeBasedLimiter.grabQuota}} only consumes \{{readCapacityUnitLimiter}} when
the
{\{readCapacityUnit}} argument is positive, so with exceed-throttle on and
capacity-unit quotas set
on the region server:
* A pure read (\{{writeCapacityUnitConsumed == 0}}) debits *zero* read capacity
units. The quota is
checked against the correct estimate in step 2 but never charged in step 4,
so read CU throttling
does not throttle. When \{{exceed}} is true (step 5) the same happens to the
user/table/namespace
limiters.
* A pure write debits the read CU limiter by the *write* CU estimate and the
request CU limiter
twice, so writes spuriously exhaust the read CU quota.
h3. Origin and affected versions
Introduced with the feature in HBASE-21783 (\{{9a55cbb2c1df}}, 2019-02-21,
shipped in 2.2.0) — the
original hunk already had \{{writeCapacityUnitConsumed,
writeCapacityUnitConsumed}}. HBASE-29231
reformatted the lines in 2025 and kept the duplicate. Present on \{{master}},
\{{branch-3}},
{\{branch-2}}, \{{branch-2.6}} and \{{rel/2.6.6}}.
{\{TestQuotaThrottle.testExceedThrottleQuota}} only exercises \{{READ_NUMBER}}
/ \{{WRITE_NUMBER}},
and the capacity-unit tests never enable exceed-throttle, which is presumably
why this has not been
noticed.
h3. Reproducer
A unit test in the style of \{{TestDefaultOperationQuota}}: region-server
limiter with
{\{READ_CAPACITY_UNIT}} soft limit 100/s and an unlimited
\{{WRITE_CAPACITY_UNIT}}, an
{\{ExceedOperationQuota}} with no other limiters, two consecutive
\{{checkBatchQuota(0, 1, false)}}
calls. One read is estimated at one 65536-byte block = 64 CU at the default
1024-byte CU size, so
the first read fits and should leave 36 CU; the second needs 64 and must throw
{\{RpcThrottlingException}}. On current master the limiter is debited 0 read CU
and the second call
passes:
{noformat}
Tests run: 14, Failures: 1, Errors: 0, Skipped: 0
TestDefaultOperationQuota.testExceedQuotaGrabsReadCapacityUnits:318
Expected org.apache.hadoop.hbase.quotas.RpcThrottlingException to be thrown,
but nothing was thrown.
{noformat}
With the two-line fix below the same run is 14/14 green (the 13 existing tests
pass in both runs).
Verified on \{{master}} at \{{09474ab77b}} with JDK 21.
{code:java}
@Test
public void testExceedQuotaGrabsReadCapacityUnits() throws
RpcThrottlingException {
QuotaProtos.Throttle rsThrottle = QuotaProtos.Throttle.newBuilder()
.setReadCapacityUnit(QuotaProtos.TimedQuota.newBuilder().setSoftLimit(100)
.setTimeUnit(HBaseProtos.TimeUnit.SECONDS).build())
.setWriteCapacityUnit(QuotaProtos.TimedQuota.newBuilder().setSoftLimit(Long.MAX_VALUE)
.setTimeUnit(HBaseProtos.TimeUnit.SECONDS).build())
.build();
QuotaLimiter rsLimiter = TimeBasedLimiter.fromThrottle(conf, rsThrottle);
// no user/table/namespace limiters: only the region server quota applies
ExceedOperationQuota quota =
new ExceedOperationQuota(new Configuration(), 65536,
DEFAULT_REQUESTS_PER_SECOND, rsLimiter);
// one read = one 65536-byte block = 64 read CU; fits in 100 and must leave 36
quota.checkBatchQuota(0, 1, false);
// the second read needs 64 CU but only 36 remain, so it must be throttled
assertThrows(RpcThrottlingException.class, () -> quota.checkBatchQuota(0, 1,
false));
}
{code}
h3. Proposed fix
{code}
- writeCapacityUnitConsumed, writeCapacityUnitConsumed, isAtomic,
handlerUsageTimeConsumed);
+ writeCapacityUnitConsumed, readCapacityUnitConsumed, isAtomic,
handlerUsageTimeConsumed);
...
- writeCapacityUnitConsumed, writeCapacityUnitConsumed, isAtomic,
0L);
+ writeCapacityUnitConsumed, readCapacityUnitConsumed, isAtomic, 0L);
{code}
I ran only this test class, not the full suite or the integration test
\{{TestQuotaThrottle}}.
Happy to open a PR with the fix and the test if that is welcome.
h3. Provenance
This was found during an academic study of LLM-assisted code review at Jeonbuk
National University, Jeonju, South Korea. An automated LLM-based reviewer
produced the initial analysis and I verified it by reading the source at
\{{09474ab77bfa}} and the \{{QuotaLimiter}} signature, and by running the unit
test above against the unpatched and patched class. I believe it is a genuine
defect and will respect your triage either way.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
