From 764f7a405b1142c67814a2c210e3f45339db3cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Paksy?= Date: Wed, 4 Dec 2024 16:06:11 +0100 Subject: [PATCH 1/3] HBASE-28999 Return HTTP 503 when stateless scan REST endpoint is invoked on disabled table Before it returned HTTP 500 Internal Server Error. --- .../rest/ServiceUnavailableException.java | 28 +++++++++++++++++++ .../hadoop/hbase/rest/TableResource.java | 3 ++ .../hadoop/hbase/rest/TestTableResource.java | 12 ++++++++ 3 files changed, 43 insertions(+) create mode 100644 hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ServiceUnavailableException.java diff --git a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ServiceUnavailableException.java b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ServiceUnavailableException.java new file mode 100644 index 000000000000..46b3e5ed14af --- /dev/null +++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ServiceUnavailableException.java @@ -0,0 +1,28 @@ +/* + * 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.rest; + +import java.io.IOException; +import org.apache.yetus.audience.InterfaceAudience; + +@InterfaceAudience.Private +public class ServiceUnavailableException extends IOException { + public ServiceUnavailableException(final String msg) { + super(msg); + } +} diff --git a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java index 2c56706e06df..5d18a8594a9a 100644 --- a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java +++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java @@ -139,6 +139,9 @@ public TableScanResource getScanResource(final @PathParam("scanspec") String sca @DefaultValue("true") @QueryParam(Constants.SCAN_INCLUDE_START_ROW) boolean includeStartRow, @DefaultValue("false") @QueryParam(Constants.SCAN_INCLUDE_STOP_ROW) boolean includeStopRow) { try { + if (servlet.getAdmin().isTableDisabled(TableName.valueOf(this.table))) { + throw new ServiceUnavailableException("Table disabled."); + } Filter prefixFilter = null; Scan tableScan = new Scan(); if (scanSpec.indexOf('*') > 0) { diff --git a/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestTableResource.java b/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestTableResource.java index c27d8ee2347a..decf8acb45c3 100644 --- a/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestTableResource.java +++ b/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestTableResource.java @@ -71,6 +71,8 @@ public class TestTableResource { private static final String COLUMN_FAMILY = "test"; private static final String COLUMN = COLUMN_FAMILY + ":qualifier"; private static final int NUM_REGIONS = 4; + private static final String DISABLED_TABLE_NAME = "disabled"; + private static final TableName DISABLED_TABLE = TableName.valueOf(DISABLED_TABLE_NAME); private static List regionMap; private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil(); @@ -117,6 +119,10 @@ public static void setUpBeforeClass() throws Exception { regionMap = m; LOG.error("regions: " + regionMap); regionLocator.close(); + + TEST_UTIL.createTable(DISABLED_TABLE, "cf"); + TEST_UTIL.getAdmin().disableTable(DISABLED_TABLE); + TEST_UTIL.waitTableDisabled(DISABLED_TABLE, 30); } @AfterClass @@ -262,4 +268,10 @@ public void testTableNotFound() throws IOException { assertEquals(404, response2.getCode()); } + @Test + public void testDisabledTableScan() throws IOException { + Response response = client.get("/" + DISABLED_TABLE_NAME + "/*", Constants.MIMETYPE_JSON); + assertEquals(503, response.getCode()); + assertTrue(Bytes.toString(response.getBody()).contains("Table disabled.")); + } } From 9f25e0357b95415cc0e1699029e09bd5497aeb8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Paksy?= Date: Wed, 4 Dec 2024 16:31:09 +0100 Subject: [PATCH 2/3] HBASE-28999 Changed table field to be private final in TableResource --- .../main/java/org/apache/hadoop/hbase/rest/TableResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java index 5d18a8594a9a..0ea74aedac59 100644 --- a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java +++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java @@ -43,7 +43,7 @@ @InterfaceAudience.Private public class TableResource extends ResourceBase { - String table; + private final String table; private static final Logger LOG = LoggerFactory.getLogger(TableResource.class); private static final Decoder base64Urldecoder = java.util.Base64.getUrlDecoder(); From cfb5623dfed0a4f28e923ae3a306963066c9391c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Paksy?= Date: Thu, 5 Dec 2024 13:21:05 +0100 Subject: [PATCH 3/3] HBASE-28525 Trying to catch TableNotEnabledException instead of eagerly checking if table is disabled. Reason: explicit check may cause some extra network traffic. --- .../hadoop/hbase/rest/ResourceBase.java | 10 +++ .../hadoop/hbase/rest/TableResource.java | 3 - .../hadoop/hbase/rest/TableScanResource.java | 61 +++++++++++-------- 3 files changed, 44 insertions(+), 30 deletions(-) diff --git a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ResourceBase.java b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ResourceBase.java index 422f4a3b9430..a37fe3a350f7 100644 --- a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ResourceBase.java +++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/ResourceBase.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.rest; import java.io.IOException; +import org.apache.hadoop.hbase.TableNotEnabledException; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.client.RetriesExhaustedException; import org.apache.hadoop.hbase.regionserver.NoSuchColumnFamilyException; @@ -74,6 +75,15 @@ protected Response processException(Throwable exp) { RetriesExhaustedException retryException = (RetriesExhaustedException) exp; processException(retryException.getCause()); } + if (exp instanceof TableNotEnabledException) { + throwServiceUnavailableException(exp); + } + + throwServiceUnavailableException(exp); + return null; + } + + private static void throwServiceUnavailableException(Throwable exp) { throw new WebApplicationException( Response.status(Response.Status.SERVICE_UNAVAILABLE).type(MIMETYPE_TEXT) .entity("Unavailable" + CRLF + StringUtils.stringifyException(exp) + CRLF).build()); diff --git a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java index 0ea74aedac59..d77efe817789 100644 --- a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java +++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableResource.java @@ -139,9 +139,6 @@ public TableScanResource getScanResource(final @PathParam("scanspec") String sca @DefaultValue("true") @QueryParam(Constants.SCAN_INCLUDE_START_ROW) boolean includeStartRow, @DefaultValue("false") @QueryParam(Constants.SCAN_INCLUDE_STOP_ROW) boolean includeStopRow) { try { - if (servlet.getAdmin().isTableDisabled(TableName.valueOf(this.table))) { - throw new ServiceUnavailableException("Table disabled."); - } Filter prefixFilter = null; Scan tableScan = new Scan(); if (scanSpec.indexOf('*') > 0) { diff --git a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableScanResource.java b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableScanResource.java index 4bb30b0cc3c7..866ad86fac16 100644 --- a/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableScanResource.java +++ b/hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/TableScanResource.java @@ -63,36 +63,43 @@ public CellSetModelStream get(final @Context UriInfo uriInfo) { LOG.trace("GET " + uriInfo.getAbsolutePath()); } servlet.getMetrics().incrementRequests(1); - final int rowsToSend = userRequestedLimit; - servlet.getMetrics().incrementSucessfulScanRequests(1); - final Iterator itr = results.iterator(); - return new CellSetModelStream(new ArrayList() { - @Override - public Iterator iterator() { - return new Iterator() { - int count = rowsToSend; - - @Override - public boolean hasNext() { - return count > 0 && itr.hasNext(); - } + try { + final int rowsToSend = userRequestedLimit; + servlet.getMetrics().incrementSucessfulScanRequests(1); + final Iterator itr = results.iterator(); + return new CellSetModelStream(new ArrayList() { + @Override + public Iterator iterator() { + return new Iterator() { + int count = rowsToSend; - @Override - public RowModel next() { - Result rs = itr.next(); - if ((rs == null) || (count <= 0)) { - return null; + @Override + public boolean hasNext() { + return count > 0 && itr.hasNext(); } - RowModel rModel = RestUtil.createRowModelFromResult(rs); - count--; - if (count == 0) { - results.close(); + + @Override + public RowModel next() { + Result rs = itr.next(); + if ((rs == null) || (count <= 0)) { + return null; + } + RowModel rModel = RestUtil.createRowModelFromResult(rs); + count--; + if (count == 0) { + results.close(); + } + return rModel; } - return rModel; - } - }; - } - }); + }; + } + }); + } catch (Exception exp) { + servlet.getMetrics().incrementFailedScanRequests(1); + processException(exp); + LOG.warn(exp.toString(), exp); + return null; + } } @GET