You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "rmdmattingly (via GitHub)" <gi...@apache.org> on 2024/02/07 18:18:45 UTC
[PR] HBASE-28349 Count atomic operations against read quotas [hbase]
rmdmattingly opened a new pull request, #5668:
URL: https://github.com/apache/hbase/pull/5668
Right now atomic operations are just treated as a single write from the quota perspective. Since an atomic operation also encompasses a read, it would make sense to increment readNum and readSize counts appropriately.
cc @bbeaudreault @hgromer @eab148 @bozzkar
--
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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5668:
URL: https://github.com/apache/hbase/pull/5668#issuecomment-1932680943
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 53s | 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 | 4m 21s | master passed |
| +1 :green_heart: | compile | 3m 7s | master passed |
| +1 :green_heart: | checkstyle | 0m 40s | master passed |
| +1 :green_heart: | spotless | 0m 51s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 49s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 55s | the patch passed |
| +1 :green_heart: | compile | 3m 4s | the patch passed |
| +1 :green_heart: | javac | 3m 4s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 37s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 6m 27s | Patch does not cause any errors with Hadoop 3.3.6. |
| +1 :green_heart: | spotless | 1m 4s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 2m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 17s | The patch does not generate ASF License warnings. |
| | | 38m 45s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5668 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 78d1af56f639 5.4.0-169-generic #187-Ubuntu SMP Thu Nov 23 14:52:28 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 275d928a7d |
| 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-5668/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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5668:
URL: https://github.com/apache/hbase/pull/5668#issuecomment-1934908132
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 9s | master passed |
| +1 :green_heart: | compile | 0m 50s | master passed |
| +1 :green_heart: | shadedjars | 5m 49s | 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 43s | the patch passed |
| +1 :green_heart: | compile | 0m 53s | the patch passed |
| +1 :green_heart: | javac | 0m 53s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 46s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 28s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 272m 9s | hbase-server in the patch failed. |
| | | 297m 16s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5668 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 022e79aeb678 5.4.0-169-generic #187-Ubuntu SMP Thu Nov 23 14:52:28 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / e85557a34d |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/3/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-5668/3/testReport/ |
| Max. process+thread count | 4767 (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-5668/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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5668:
URL: https://github.com/apache/hbase/pull/5668#issuecomment-1934470619
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 23s | 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 42s | master passed |
| +1 :green_heart: | compile | 2m 29s | master passed |
| +1 :green_heart: | checkstyle | 0m 33s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 25s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 44s | the patch passed |
| +1 :green_heart: | compile | 2m 28s | the patch passed |
| +1 :green_heart: | javac | 2m 28s | the patch passed |
| +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 | 5m 2s | Patch does not cause any errors with Hadoop 3.3.6. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 34s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 8s | The patch does not generate ASF License warnings. |
| | | 27m 45s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5668 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux c9a335704284 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / e85557a34d |
| 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-5668/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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5668:
URL: https://github.com/apache/hbase/pull/5668#discussion_r1481954598
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestAtomicReadQuota.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.quotas;
+
+import java.io.IOException;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.Increment;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.After;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({ RegionServerTests.class, MediumTests.class })
+public class TestAtomicReadQuota {
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestAtomicReadQuota.class);
+ private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
+ private static final TableName TABLE_NAME = TableName.valueOf(UUID.randomUUID().toString());
+ private static final byte[] FAMILY = Bytes.toBytes("cf");
+ private static final byte[] QUALIFIER = Bytes.toBytes("q");
+
+ @After
+ public void tearDown() throws Exception {
+ ThrottleQuotaTestUtil.clearQuotaCache(TEST_UTIL);
+ EnvironmentEdgeManager.reset();
+ TEST_UTIL.deleteTable(TABLE_NAME);
+ TEST_UTIL.shutdownMiniCluster();
+ }
+
+ @BeforeClass
+ public static void setUpBeforeClass() throws Exception {
+ TEST_UTIL.getConfiguration().setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ TEST_UTIL.getConfiguration().setInt(QuotaCache.REFRESH_CONF_KEY, 1000);
+ TEST_UTIL.startMiniCluster(1);
+ TEST_UTIL.waitTableAvailable(QuotaTableUtil.QUOTA_TABLE_NAME);
+ TEST_UTIL.createTable(TABLE_NAME, FAMILY);
+ TEST_UTIL.waitTableAvailable(TABLE_NAME);
+ QuotaCache.TEST_FORCE_REFRESH = true;
+ }
+
+ @Test
+ public void testAtomicOpCountedAgainstReadCapacity() throws Exception {
Review Comment:
it's probably worth having a test for checkAndMutate as well if we're going to go to the trouble 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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5668:
URL: https://github.com/apache/hbase/pull/5668#discussion_r1482991329
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java:
##########
@@ -2679,7 +2679,8 @@ public MultiResponse multi(final RpcController rpcc, final MultiRequest request)
try {
region = getRegion(regionSpecifier);
- quota = getRpcQuotaManager().checkQuota(region, regionAction.getActionList());
+ quota = getRpcQuotaManager().checkQuota(region, regionAction.getActionList(),
+ regionAction.getAtomic());
Review Comment:
per my other comment about RowMutations I think we actually want to change this and the other usage of getAtomic to hasCondition. I'm realizing that getAtomic seems specific to RowMutations, which may or may not be conditional depending of if the RowMutations is called via mutateRow, batch, or checkAndMutate (only the last one is conditional).
With this, we'll need to update your RowMutations test to ensure that it doesnt count against read capacity unless its a checkAndMutate
--
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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5668:
URL: https://github.com/apache/hbase/pull/5668#discussion_r1483180170
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestAtomicReadQuota.java:
##########
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.quotas;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.CheckAndMutate;
+import org.apache.hadoop.hbase.client.Increment;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.RowMutations;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Category({ RegionServerTests.class, MediumTests.class })
+public class TestAtomicReadQuota {
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestAtomicReadQuota.class);
+ private static final Logger LOG = LoggerFactory.getLogger(TestAtomicReadQuota.class);
+ private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
+ private static final TableName TABLE_NAME = TableName.valueOf(UUID.randomUUID().toString());
+ private static final byte[] FAMILY = Bytes.toBytes("cf");
+ private static final byte[] QUALIFIER = Bytes.toBytes("q");
+
+ @AfterClass
+ public static void tearDown() throws Exception {
+ ThrottleQuotaTestUtil.clearQuotaCache(TEST_UTIL);
+ EnvironmentEdgeManager.reset();
+ TEST_UTIL.deleteTable(TABLE_NAME);
+ TEST_UTIL.shutdownMiniCluster();
+ }
+
+ @BeforeClass
+ public static void setUpBeforeClass() throws Exception {
+ TEST_UTIL.getConfiguration().setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ TEST_UTIL.getConfiguration().setInt(QuotaCache.REFRESH_CONF_KEY, 1000);
+ TEST_UTIL.startMiniCluster(1);
+ TEST_UTIL.waitTableAvailable(QuotaTableUtil.QUOTA_TABLE_NAME);
+ TEST_UTIL.createTable(TABLE_NAME, FAMILY);
+ TEST_UTIL.waitTableAvailable(TABLE_NAME);
+ QuotaCache.TEST_FORCE_REFRESH = true;
+ }
+
+ @Test
+ public void testIncrementCountedAgainstReadCapacity() throws Exception {
+ setupQuota();
+
+ Increment inc = new Increment(Bytes.toBytes(UUID.randomUUID().toString()));
+ inc.addColumn(FAMILY, QUALIFIER, 1);
+ testThrottle(table -> table.increment(inc));
+ }
+
+ @Test
+ public void testRowMutationsCountedAgainstReadCapacity() throws Exception {
+ setupQuota();
+
+ byte[] row = Bytes.toBytes(UUID.randomUUID().toString());
+ Put put = new Put(row);
+ put.addColumn(FAMILY, Bytes.toBytes("doot"), Bytes.toBytes("v"));
+
+ RowMutations rowMutations = new RowMutations(row);
+ rowMutations.add(put);
+ testThrottle(table -> table.mutateRow(rowMutations));
Review Comment:
That is interesting. I haven't really tested that it works, but it doesn't throw an exception
--
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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5668:
URL: https://github.com/apache/hbase/pull/5668#discussion_r1481950061
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/RegionServerRpcQuotaManager.java:
##########
@@ -157,39 +157,44 @@ public OperationQuota getQuota(final UserGroupInformation ugi, final TableName t
/**
* Check the quota for the current (rpc-context) user. Returns the OperationQuota used to get the
* available quota and to report the data/usage of the operation.
- * @param region the region where the operation will be performed
- * @param type the operation type
+ * @param region the region where the operation will be performed
+ * @param type the operation type
+ * @param isAtomic whether the given operation is atomic
* @return the OperationQuota
* @throws RpcThrottlingException if the operation cannot be executed due to quota exceeded.
*/
- public OperationQuota checkQuota(final Region region, final OperationQuota.OperationType type)
- throws IOException, RpcThrottlingException {
+ public OperationQuota checkQuota(final Region region, final OperationQuota.OperationType type,
+ boolean isAtomic) throws IOException, RpcThrottlingException {
switch (type) {
case SCAN:
return checkQuota(region, 0, 0, 1);
case GET:
return checkQuota(region, 0, 1, 0);
case MUTATE:
- return checkQuota(region, 1, 0, 0);
+ return checkQuota(region, 1, isAtomic ? 1 : 0, 0);
}
throw new RuntimeException("Invalid operation type: " + type);
}
/**
* Check the quota for the current (rpc-context) user. Returns the OperationQuota used to get the
* available quota and to report the data/usage of the operation.
- * @param region the region where the operation will be performed
- * @param actions the "multi" actions to perform
+ * @param region the region where the operation will be performed
+ * @param actions the "multi" actions to perform
+ * @param isAtomic whether the given operation is atomic
* @return the OperationQuota
* @throws RpcThrottlingException if the operation cannot be executed due to quota exceeded.
*/
- public OperationQuota checkQuota(final Region region, final List<ClientProtos.Action> actions)
- throws IOException, RpcThrottlingException {
+ public OperationQuota checkQuota(final Region region, final List<ClientProtos.Action> actions,
+ boolean isAtomic) throws IOException, RpcThrottlingException {
int numWrites = 0;
int numReads = 0;
for (final ClientProtos.Action action : actions) {
if (action.hasMutation()) {
numWrites++;
+ if (isAtomic) {
Review Comment:
i think you should also check the action MutateType here. We want to treat Increment and Append as reads as well, but those don't have to come from an atomic RegionAction
--
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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5668:
URL: https://github.com/apache/hbase/pull/5668#issuecomment-1934161039
: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 | 2m 59s | master passed |
| +1 :green_heart: | compile | 2m 30s | master passed |
| +1 :green_heart: | checkstyle | 0m 33s | master passed |
| +1 :green_heart: | spotless | 0m 42s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 27s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 44s | the patch passed |
| +1 :green_heart: | compile | 2m 27s | the patch passed |
| +1 :green_heart: | javac | 2m 27s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 34s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 5m 4s | Patch does not cause any errors with Hadoop 3.3.6. |
| +1 :green_heart: | spotless | 0m 40s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | The patch does not generate ASF License warnings. |
| | | 28m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5668 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 19b37956a342 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 275d928a7d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 77 (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-5668/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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5668:
URL: https://github.com/apache/hbase/pull/5668#discussion_r1483081909
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestAtomicReadQuota.java:
##########
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.quotas;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.CheckAndMutate;
+import org.apache.hadoop.hbase.client.Increment;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.RowMutations;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Category({ RegionServerTests.class, MediumTests.class })
+public class TestAtomicReadQuota {
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestAtomicReadQuota.class);
+ private static final Logger LOG = LoggerFactory.getLogger(TestAtomicReadQuota.class);
+ private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
+ private static final TableName TABLE_NAME = TableName.valueOf(UUID.randomUUID().toString());
+ private static final byte[] FAMILY = Bytes.toBytes("cf");
+ private static final byte[] QUALIFIER = Bytes.toBytes("q");
+
+ @AfterClass
+ public static void tearDown() throws Exception {
+ ThrottleQuotaTestUtil.clearQuotaCache(TEST_UTIL);
+ EnvironmentEdgeManager.reset();
+ TEST_UTIL.deleteTable(TABLE_NAME);
+ TEST_UTIL.shutdownMiniCluster();
+ }
+
+ @BeforeClass
+ public static void setUpBeforeClass() throws Exception {
+ TEST_UTIL.getConfiguration().setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ TEST_UTIL.getConfiguration().setInt(QuotaCache.REFRESH_CONF_KEY, 1000);
+ TEST_UTIL.startMiniCluster(1);
+ TEST_UTIL.waitTableAvailable(QuotaTableUtil.QUOTA_TABLE_NAME);
+ TEST_UTIL.createTable(TABLE_NAME, FAMILY);
+ TEST_UTIL.waitTableAvailable(TABLE_NAME);
+ QuotaCache.TEST_FORCE_REFRESH = true;
+ }
+
+ @Test
+ public void testIncrementCountedAgainstReadCapacity() throws Exception {
+ setupQuota();
+
+ Increment inc = new Increment(Bytes.toBytes(UUID.randomUUID().toString()));
+ inc.addColumn(FAMILY, QUALIFIER, 1);
+ testThrottle(table -> table.increment(inc));
+ }
+
+ @Test
+ public void testRowMutationsCountedAgainstReadCapacity() throws Exception {
+ setupQuota();
+
+ byte[] row = Bytes.toBytes(UUID.randomUUID().toString());
+ Put put = new Put(row);
+ put.addColumn(FAMILY, Bytes.toBytes("doot"), Bytes.toBytes("v"));
+
+ RowMutations rowMutations = new RowMutations(row);
+ rowMutations.add(put);
+ testThrottle(table -> table.mutateRow(rowMutations));
Review Comment:
This test confirms that this is rightfully counted against read capacity now, and I added the test `testNonConditionalRowMutationsOmittedFromReadCapacity` test to confirm the case where non-conditional, non-atomic, ops are passed in to a RowMutations
--
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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5668:
URL: https://github.com/apache/hbase/pull/5668#issuecomment-1934933972
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 42s | 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 | 3m 33s | master passed |
| +1 :green_heart: | compile | 1m 1s | master passed |
| +1 :green_heart: | shadedjars | 6m 4s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 29s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 52s | 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 | 6m 49s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 288m 18s | hbase-server in the patch passed. |
| | | 316m 58s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5668 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux fc3cfd8c4994 5.4.0-166-generic #183-Ubuntu SMP Mon Oct 2 11:28:33 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / e85557a34d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/3/testReport/ |
| Max. process+thread count | 4945 (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-5668/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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5668:
URL: https://github.com/apache/hbase/pull/5668#discussion_r1482991617
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestAtomicReadQuota.java:
##########
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.quotas;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.CheckAndMutate;
+import org.apache.hadoop.hbase.client.Increment;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.RowMutations;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Category({ RegionServerTests.class, MediumTests.class })
+public class TestAtomicReadQuota {
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestAtomicReadQuota.class);
+ private static final Logger LOG = LoggerFactory.getLogger(TestAtomicReadQuota.class);
+ private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
+ private static final TableName TABLE_NAME = TableName.valueOf(UUID.randomUUID().toString());
+ private static final byte[] FAMILY = Bytes.toBytes("cf");
+ private static final byte[] QUALIFIER = Bytes.toBytes("q");
+
+ @AfterClass
+ public static void tearDown() throws Exception {
+ ThrottleQuotaTestUtil.clearQuotaCache(TEST_UTIL);
+ EnvironmentEdgeManager.reset();
+ TEST_UTIL.deleteTable(TABLE_NAME);
+ TEST_UTIL.shutdownMiniCluster();
+ }
+
+ @BeforeClass
+ public static void setUpBeforeClass() throws Exception {
+ TEST_UTIL.getConfiguration().setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ TEST_UTIL.getConfiguration().setInt(QuotaCache.REFRESH_CONF_KEY, 1000);
+ TEST_UTIL.startMiniCluster(1);
+ TEST_UTIL.waitTableAvailable(QuotaTableUtil.QUOTA_TABLE_NAME);
+ TEST_UTIL.createTable(TABLE_NAME, FAMILY);
+ TEST_UTIL.waitTableAvailable(TABLE_NAME);
+ QuotaCache.TEST_FORCE_REFRESH = true;
+ }
+
+ @Test
+ public void testIncrementCountedAgainstReadCapacity() throws Exception {
+ setupQuota();
+
+ Increment inc = new Increment(Bytes.toBytes(UUID.randomUUID().toString()));
+ inc.addColumn(FAMILY, QUALIFIER, 1);
+ testThrottle(table -> table.increment(inc));
+ }
+
+ @Test
+ public void testRowMutationsCountedAgainstReadCapacity() throws Exception {
+ setupQuota();
+
+ byte[] row = Bytes.toBytes(UUID.randomUUID().toString());
+ Put put = new Put(row);
+ put.addColumn(FAMILY, Bytes.toBytes("doot"), Bytes.toBytes("v"));
+
+ RowMutations rowMutations = new RowMutations(row);
+ rowMutations.add(put);
+ testThrottle(table -> table.mutateRow(rowMutations));
Review Comment:
Per my other comments, this should not count against read capacity.
--
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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault merged PR #5668:
URL: https://github.com/apache/hbase/pull/5668
--
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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5668:
URL: https://github.com/apache/hbase/pull/5668#discussion_r1482984152
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/RegionServerRpcQuotaManager.java:
##########
@@ -157,39 +157,48 @@ public OperationQuota getQuota(final UserGroupInformation ugi, final TableName t
/**
* Check the quota for the current (rpc-context) user. Returns the OperationQuota used to get the
* available quota and to report the data/usage of the operation.
- * @param region the region where the operation will be performed
- * @param type the operation type
+ * @param region the region where the operation will be performed
+ * @param type the operation type
+ * @param isAtomic whether the given operation is atomic
* @return the OperationQuota
* @throws RpcThrottlingException if the operation cannot be executed due to quota exceeded.
*/
- public OperationQuota checkQuota(final Region region, final OperationQuota.OperationType type)
- throws IOException, RpcThrottlingException {
+ public OperationQuota checkQuota(final Region region, final OperationQuota.OperationType type,
Review Comment:
I'm thinking we should add another OperationType CHECK_AND_MUTATE. We can pass that in here instead of adding an isAtomic boolean. We can also add a util in QuotaUtil to return an OperationType for an Action + hasCondition boolean.
We can then use that same api in the other checkQuota below. Something like:
```java
for (final ClientProtos.Action action : actions) {
OperationType type = QuotaUtil.getOperationType(action, hasCondition);
if (type.isRead()) {
numReads++;
}
if (type.isWrite()) {
numWrites++;
}
}
```
The goal with this change is to DRY up the mutation type checking, and also to clarify a bit. Currently we are using the `isAtomic` boolean name which I'm realizing is not accurate. `RowMutations` is an example of an atomic operation which is not conditional (thus has no reads).
--
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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5668:
URL: https://github.com/apache/hbase/pull/5668#issuecomment-1933047037
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 25s | 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 | 3m 19s | master passed |
| +1 :green_heart: | compile | 0m 47s | master passed |
| +1 :green_heart: | shadedjars | 5m 49s | 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 57s | 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 | 5m 46s | 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 | 227m 46s | hbase-server in the patch passed. |
| | | 252m 41s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5668 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux fd664535b853 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 275d928a7d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/1/testReport/ |
| Max. process+thread count | 5490 (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-5668/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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5668:
URL: https://github.com/apache/hbase/pull/5668#issuecomment-1933061601
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 44s | 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 | 3m 58s | master passed |
| +1 :green_heart: | compile | 0m 48s | master passed |
| +1 :green_heart: | shadedjars | 5m 59s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 29s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 34s | the patch passed |
| +1 :green_heart: | compile | 0m 53s | the patch passed |
| +1 :green_heart: | javac | 0m 53s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 56s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 239m 33s | hbase-server in the patch passed. |
| | | 266m 44s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5668 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0ec3c182a529 5.4.0-169-generic #187-Ubuntu SMP Thu Nov 23 14:52:28 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 275d928a7d |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5668/1/testReport/ |
| Max. process+thread count | 4980 (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-5668/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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5668:
URL: https://github.com/apache/hbase/pull/5668#discussion_r1483096590
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestAtomicReadQuota.java:
##########
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.quotas;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.CheckAndMutate;
+import org.apache.hadoop.hbase.client.Increment;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.RowMutations;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Category({ RegionServerTests.class, MediumTests.class })
+public class TestAtomicReadQuota {
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestAtomicReadQuota.class);
+ private static final Logger LOG = LoggerFactory.getLogger(TestAtomicReadQuota.class);
+ private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
+ private static final TableName TABLE_NAME = TableName.valueOf(UUID.randomUUID().toString());
+ private static final byte[] FAMILY = Bytes.toBytes("cf");
+ private static final byte[] QUALIFIER = Bytes.toBytes("q");
+
+ @AfterClass
+ public static void tearDown() throws Exception {
+ ThrottleQuotaTestUtil.clearQuotaCache(TEST_UTIL);
+ EnvironmentEdgeManager.reset();
+ TEST_UTIL.deleteTable(TABLE_NAME);
+ TEST_UTIL.shutdownMiniCluster();
+ }
+
+ @BeforeClass
+ public static void setUpBeforeClass() throws Exception {
+ TEST_UTIL.getConfiguration().setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ TEST_UTIL.getConfiguration().setInt(QuotaCache.REFRESH_CONF_KEY, 1000);
+ TEST_UTIL.startMiniCluster(1);
+ TEST_UTIL.waitTableAvailable(QuotaTableUtil.QUOTA_TABLE_NAME);
+ TEST_UTIL.createTable(TABLE_NAME, FAMILY);
+ TEST_UTIL.waitTableAvailable(TABLE_NAME);
+ QuotaCache.TEST_FORCE_REFRESH = true;
+ }
+
+ @Test
+ public void testIncrementCountedAgainstReadCapacity() throws Exception {
+ setupQuota();
+
+ Increment inc = new Increment(Bytes.toBytes(UUID.randomUUID().toString()));
+ inc.addColumn(FAMILY, QUALIFIER, 1);
+ testThrottle(table -> table.increment(inc));
+ }
+
+ @Test
+ public void testRowMutationsCountedAgainstReadCapacity() throws Exception {
+ setupQuota();
+
+ byte[] row = Bytes.toBytes(UUID.randomUUID().toString());
+ Put put = new Put(row);
+ put.addColumn(FAMILY, Bytes.toBytes("doot"), Bytes.toBytes("v"));
+
+ RowMutations rowMutations = new RowMutations(row);
+ rowMutations.add(put);
+ testThrottle(table -> table.mutateRow(rowMutations));
Review Comment:
Interesting, I'm actually surprised it allows increment to be added to the RowMutations. According to the javadoc:
```
/**
* Currently only supports {@link Put} and {@link Delete} mutations.
* @param mutation The data to send.
* @throws IOException if the row of added mutation doesn't match the original row
*/
public RowMutations add(Mutation mutation) throws IOException {
```
--
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
Re: [PR] HBASE-28349 Count atomic operations against read quotas [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5668:
URL: https://github.com/apache/hbase/pull/5668#discussion_r1483194877
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestAtomicReadQuota.java:
##########
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.quotas;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.CheckAndMutate;
+import org.apache.hadoop.hbase.client.Increment;
+import org.apache.hadoop.hbase.client.Put;
+import org.apache.hadoop.hbase.client.RowMutations;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Category({ RegionServerTests.class, MediumTests.class })
+public class TestAtomicReadQuota {
+ @ClassRule
+ public static final HBaseClassTestRule CLASS_RULE =
+ HBaseClassTestRule.forClass(TestAtomicReadQuota.class);
+ private static final Logger LOG = LoggerFactory.getLogger(TestAtomicReadQuota.class);
+ private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
+ private static final TableName TABLE_NAME = TableName.valueOf(UUID.randomUUID().toString());
+ private static final byte[] FAMILY = Bytes.toBytes("cf");
+ private static final byte[] QUALIFIER = Bytes.toBytes("q");
+
+ @AfterClass
+ public static void tearDown() throws Exception {
+ ThrottleQuotaTestUtil.clearQuotaCache(TEST_UTIL);
+ EnvironmentEdgeManager.reset();
+ TEST_UTIL.deleteTable(TABLE_NAME);
+ TEST_UTIL.shutdownMiniCluster();
+ }
+
+ @BeforeClass
+ public static void setUpBeforeClass() throws Exception {
+ TEST_UTIL.getConfiguration().setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ TEST_UTIL.getConfiguration().setInt(QuotaCache.REFRESH_CONF_KEY, 1000);
+ TEST_UTIL.startMiniCluster(1);
+ TEST_UTIL.waitTableAvailable(QuotaTableUtil.QUOTA_TABLE_NAME);
+ TEST_UTIL.createTable(TABLE_NAME, FAMILY);
+ TEST_UTIL.waitTableAvailable(TABLE_NAME);
+ QuotaCache.TEST_FORCE_REFRESH = true;
+ }
+
+ @Test
+ public void testIncrementCountedAgainstReadCapacity() throws Exception {
+ setupQuota();
+
+ Increment inc = new Increment(Bytes.toBytes(UUID.randomUUID().toString()));
+ inc.addColumn(FAMILY, QUALIFIER, 1);
+ testThrottle(table -> table.increment(inc));
+ }
+
+ @Test
+ public void testRowMutationsCountedAgainstReadCapacity() throws Exception {
+ setupQuota();
+
+ byte[] row = Bytes.toBytes(UUID.randomUUID().toString());
+ Put put = new Put(row);
+ put.addColumn(FAMILY, Bytes.toBytes("doot"), Bytes.toBytes("v"));
+
+ RowMutations rowMutations = new RowMutations(row);
+ rowMutations.add(put);
+ testThrottle(table -> table.mutateRow(rowMutations));
Review Comment:
Ok, oddly this particular invocation does work. But other invocations do not. Seems unrelated to this issue though, so we can disregard.
--
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