-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Add timeout for ccr recovery action #37840
Add timeout for ccr recovery action #37840
Conversation
This is related to elastic#35975. It adds a action timeout setting that allows timeouts to be applied to the individual transport actions that are used during a ccr recovery.
Pinging @elastic/es-distributed |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code and default value looks good (we can always increase it later if we see the need for it). For testing, you could set a low timeout, and then inject a failure at an arbitrary point (i.e. randomly on one of the CCR recovery actions) using MockTransportService
, see RecoveryActionBlocker
as an example in IndexRecoveryIT
, and then check that the restore fails in a timely fashion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
settingsRequest = new ClusterUpdateSettingsRequest(); | ||
TimeValue defaultValue = CcrSettings.INDICES_RECOVERY_ACTION_TIMEOUT_SETTING.getDefault(Settings.EMPTY); | ||
settingsRequest.persistentSettings(Settings.builder().put(CcrSettings.INDICES_RECOVERY_ACTION_TIMEOUT_SETTING.getKey(), | ||
defaultValue)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setting it to null also resets to the default
@@ -308,6 +313,78 @@ public void testRateLimitingIsEmployed() throws Exception { | |||
} | |||
} | |||
|
|||
public void testIndividualActionsTimeout() throws Exception { | |||
ClusterUpdateSettingsRequest settingsRequest = new ClusterUpdateSettingsRequest(); | |||
TimeValue timeValue = new TimeValue(100); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer TimeValue.timeValueMillis(100)
. It's clearer what the 100 are.
@elasticmachine run elasticsearch-ci/2 |
This is related to elastic#35975. It adds a action timeout setting that allows timeouts to be applied to the individual transport actions that are used during a ccr recovery.
This is related to #35975. It adds a action timeout setting that allows
timeouts to be applied to the individual transport actions that are
used during a ccr recovery.