bbeaudreault commented on code in PR #4900: URL: https://github.com/apache/hbase/pull/4900#discussion_r1035481650
########## hbase-client/src/main/java/org/apache/hadoop/hbase/client/OperationTimeoutExceededException.java: ########## @@ -0,0 +1,45 @@ +/* + * 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.client; + +import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.yetus.audience.InterfaceAudience; + +/** Review Comment: the formatting (whitespace) seems weird in this class. did you run spotless? if it somehow didn't fix this, might want to clean it up manually ########## hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncRequestFutureImpl.java: ########## @@ -207,6 +207,15 @@ public void run() { // Cancelled return; } + } catch (OperationTimeoutExceededException e) { + // The operation has timed out before executing the actual callable. This may be due to Review Comment: could you fix the newlines in this comment? also it seems standard to include a pointer to the jira in comments like this. I.e. `See HBASE-27487` ########## hbase-client/src/main/java/org/apache/hadoop/hbase/client/CancellableRegionServerCallable.java: ########## @@ -64,7 +63,8 @@ public T call(int operationTimeout) throws IOException { int remainingTime = tracker.getRemainingTime(operationTimeout); if (remainingTime <= 1) { // "1" is a special return value in RetryingTimeTracker, see its implementation. - throw new DoNotRetryIOException("Operation rpcTimeout"); + throw new OperationTimeoutExceededException( + "Timeout exceeded before call began. Slow meta may be the cause or the operation timeout is too short."); Review Comment: Another reason this can occur is if simply the configured retries can't complete in the operation timeout. We might want to include that. For example, set retries to 100000 and timeout to 1s and it'd be impossible to exceed retries before timing out. In that case I think we'd fall through here. Of course an absurd example, but I think it's easy to realistically hit it as well -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
