Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HDDS-11226. Make ExponentialBackoffPolicy maxRetries configurable #6985

Merged
merged 2 commits into from
Aug 2, 2024

Conversation

ashishkumar50
Copy link
Contributor

What changes were proposed in this pull request?

Default max retries count is very high for ExponentialBackoffPolicy which in some cases leads to longer wait for client. Client like HBase by default timesout after 5 minutes which leads to unexpected crash.
In this PR making retry count as configurable so that after max retry client can try on different DN and doesn't need to wait for long.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-11226

How was this patch tested?

Updated existing test

Copy link
Contributor

@ivandika3 ivandika3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ashishkumar50 Thanks for the patch. LGTM +1.

@@ -203,6 +203,21 @@ public void setExponentialPolicyMaxSleep(Duration duration) {
exponentialPolicyMaxSleep = duration;
}

@Config(key = "client.exponential.backoff.max.retries",
defaultValue = "2147483647",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
defaultValue = "2147483647",
defaultValue = String.valueOf(Integer.MAX_VALUE),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String.valueOf doesn't return constant but defaultValue requires a constant String.
Other way we can change like "" + Integer.MAX_VALUE but this has overhead of StringBuilder.

Copy link
Contributor

@kerneltime kerneltime Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could define

  private final static string intMax = Integer.toString(Integer.MAX_VALUE);

in this class and then assign defaultValue = intMax?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this before. Tried again, but it doesn't work. Same error "Attribute value must be constant".

@adoroszlai adoroszlai changed the title HDDS-11226. Make ExponentialBackoffPolicy maxRetries as configurable. HDDS-11226. Make ExponentialBackoffPolicy maxRetries configurable Jul 26, 2024
@ashishkumar50 ashishkumar50 requested a review from kerneltime July 29, 2024 08:31
Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ashishkumar50 . lgtm

@smengcl smengcl merged commit a33af1d into apache:HDDS-7593 Aug 2, 2024
8 checks passed
errose28 added a commit to errose28/ozone that referenced this pull request Aug 7, 2024
* master: (181 commits)
  HDDS-11289. Bump docker-maven-plugin to 0.45.0 (apache#7024)
  HDDS-11287. Code cleanup in XceiverClientSpi (apache#7043)
  HDDS-11283. Refactor KeyValueStreamDataChannel to avoid spurious IDE build issues (apache#7040)
  HDDS-11257. Ozone write does not work when http proxy is set for the JVM. (apache#7036)
  HDDS-11249. Bump ozone-runner to 20240729-jdk17-1 (apache#7003)
  HDDS-10517. Recon - Add a UI component for showing DN decommissioning detailed status and info (apache#6724)
  HDDS-11270. [hsync] Add DN layout version (HBASE_SUPPORT/version 8) upgrade test. (apache#7021)
  HDDS-11272. Statistics some node status information (apache#7025)
  HDDS-11278. Move code out of Hadoop util package (apache#7028)
  HDDS-11274. (addendum) Replace Hadoop annotations/configs with Ozone-specific ones
  HDDS-11274. Replace Hadoop annotations/configs with Ozone-specific ones (apache#7026)
  HDDS-11280. Add Synchronize in AbstractCommitWatcher.addAckDataLength (apache#7032)
  HDDS-11235. Spare InfoBucket RPC call in FileSystem#mkdir() call. (apache#6990)
  HDDS-11273. Bump commons-compress to 1.26.2 (apache#7023)
  HDDS-11225. Increase ipc.server.read.threadpool.size (apache#7007)
  HDDS-11224. Increase hdds.datanode.handler.count (apache#7011)
  HDDS-11259. [hsync] DataNode should verify HBASE_SUPPORT layout version for every PutBlock. (apache#7012)
  HDDS-11214. Added config to set rocksDB's max log file size and num of log files (apache#7014)
  HDDS-11226. Make ExponentialBackoffPolicy maxRetries configurable (apache#6985)
  HDDS-11260. [hsync] Add Ozone Manager protocol version (apache#7015)
  ...

Conflicts:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/audit/DNAction.java
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestContainerReportHandler.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants