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.

Sign up for GitHub

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

CDAP-21091: Increasing timeout for task workers to recover when app fabric restarts #15855

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

CDAP-21091: Increasing timeout for task workers to recover when app fabric restarts #15855

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...src/main/java/io/cdap/cdap/runtime/spi/provisioner/dataproc/ComputeEngineCredentials.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@
/**
* Time (in millisecond) to refresh the credentials before it expires.
*/
private static final int NUMBER_OF_RETRIES = 10;
private static final int MIN_WAIT_TIME_MILLISECOND = 500;
private static final int MAX_WAIT_TIME_MILLISECOND = 10000;
private static final int NUMBER_OF_RETRIES = 20;
private static final int MIN_WAIT_TIME_MILLISECOND = 2000;
private static final int MAX_WAIT_TIME_MILLISECOND = 60000;
Comment on lines +62 to +64
Copy link
Member

Choose a reason for hiding this comment

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

can we please move these to cconf?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move only the NUMBER_OF_RETRIES to cconf.
That's the value we would need to modify mainly to configure the retry behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should keep min / max wait time the same and make NUMBER_OF_RETRIES configurable in cconf.

If later we figure out that the number of retries are insufficient, we won't have to make data plane changes for it.

private static final SecureRandom SECURE_RANDOM = new SecureRandom();
private final String endPoint;

Expand Down Expand Up @@ -102,7 +102,7 @@
}
}

private void disableVerifySSL(HttpsURLConnection connection) throws IOException {

Check warning on line 105 in cdap-runtime-ext-dataproc/src/main/java/io/cdap/cdap/runtime/spi/provisioner/dataproc/ComputeEngineCredentials.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.naming.AbbreviationAsWordInNameCheck 

Abbreviation in name 'disableVerifySSL' must contain no more than '1' consecutive capital letters.

Check warning on line 105 in cdap-runtime-ext-dataproc/src/main/java/io/cdap/cdap/runtime/spi/provisioner/dataproc/ComputeEngineCredentials.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.naming.AbbreviationAsWordInNameCheck 

Abbreviation in name 'disableVerifySSL' must contain no more than '1' consecutive capital letters.
try {
SSLContext sslContextWithNoVerify = SSLContext.getInstance("SSL");
TrustManager[] trustAllCerts = new TrustManager[]{new X509TrustManager() {
Expand All @@ -119,7 +119,7 @@
public void checkServerTrusted(X509Certificate[] arg0, String arg1) {
// No-op
}
}};

Check warning on line 122 in cdap-runtime-ext-dataproc/src/main/java/io/cdap/cdap/runtime/spi/provisioner/dataproc/ComputeEngineCredentials.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.whitespace.WhitespaceAroundCheck 

WhitespaceAround: '}' is not followed by whitespace. Empty blocks may only be represented as {} when not part of a multi-block statement (4.1.3)

Check warning on line 122 in cdap-runtime-ext-dataproc/src/main/java/io/cdap/cdap/runtime/spi/provisioner/dataproc/ComputeEngineCredentials.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.whitespace.WhitespaceAroundCheck 

WhitespaceAround: '}' is not followed by whitespace. Empty blocks may only be represented as {} when not part of a multi-block statement (4.1.3)
sslContextWithNoVerify.init(null, trustAllCerts, SECURE_RANDOM);
connection.setSSLSocketFactory(sslContextWithNoVerify.getSocketFactory());
connection.setHostnameVerifier((s, sslSession) -> true);
Expand Down