Skip to content

adding dev.Failsafe Retry policy to handle SQL transient#1

Open
AbhishekKumar9984 wants to merge 1 commit intodevelopfrom
abhishek
Open

adding dev.Failsafe Retry policy to handle SQL transient#1
AbhishekKumar9984 wants to merge 1 commit intodevelopfrom
abhishek

Conversation

@AbhishekKumar9984
Copy link
Owner

No description provided.

@vikasrathee-cs
Copy link

will check

@AbhishekKumar9984 AbhishekKumar9984 requested review from AnkitCLI and removed request for AnkitCLI April 7, 2025 10:27
@vikasrathee-cs
Copy link

@AbhishekKumar9984 PR is missing the major implementation steps, it is taking care of retries at single place that too while getting connection only, failures can be there while executing queries, getting data from resultsets and while getting schema. All those are not yet handled. All the values related to retry policy will come from either user or some default values as we have done in Salesforce, take a reference from there and then do it

Comment on lines +42 to +43


Copy link
Collaborator

Choose a reason for hiding this comment

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

No change?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove extra lines


protected void setFieldAccordingToSchema(ResultSet resultSet, StructuredRecord.Builder recordBuilder,
Schema.Field field, int columnIndex) throws SQLException {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove extra line.

// private static final String NAME_RETRY_MULTIPLIER = "retryMultiplier";
private static final String NAME_MAX_RETRY_COUNT = "maxRetryCount";
public static final int DEFAULT_INITIAL_RETRY_DURATION_SECONDS = 2;
public static final int DEFAULT_RETRY_MULTIPLIER = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

DEFAULT_RETRY_MULTIPLIER - Is this constant needed? If not, remove it.

Copy link
Owner Author

Choose a reason for hiding this comment

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

at this point it is not needed but if we want a retry multiplier which is specified according to user .so we can use it for now i will remove it .

Comment on lines 139 to 144
Failsafe.with(RetryPolicyUtil.RETRY_POLICY).run(() -> {
String[] columns = config.getArgumentsColumns().split(",");
for (String column : columns) {
arguments.set(column, resultSet.getString(column));
}
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any call to the backend taking place here that can cause SQLTransientConnectionException? If not, retry is not needed here.


@Override
public Statement createStatement() throws SQLException {

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove extra line


import dev.failsafe.Failsafe;
import io.cdap.plugin.util.RetryPolicyUtil;

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove extra line

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.

3 participants