Integ test : Stop replication when remote cluster is unreachable #69
Conversation
| } catch(e: RetentionLeaseNotFoundException) { | ||
| // log error and bail | ||
| log.error("${e.message}") | ||
| } catch (e: Exception) { | ||
| log.error("Exception in removing retention lease", e) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Can we rename the "removeRetentionLease" method to also indicate that it doesn't propagate exception?
For example - either removeRetentionLeaseNoThrow or attemptRetentionLeaseRemoval etc just so we are clear that it is only best attempt.
There was a problem hiding this comment.
Thinking out loud : how would a cluster admin know that this has happened ? I see that logs are the only current way of this . Concerned as leaking retention leases on leader side can have side effects like no truncation of translog .
There was a problem hiding this comment.
Can we add some message in the stop replication response to convey this?
The other option I can think of is let the stop replication fail by default but allow the customer to pass a flag to indicate that remote call should not be made in the stop replication call in which case the stop replication should complete without removing the retention lease.
There was a problem hiding this comment.
As discussed, this method should not throw exception as stop replication should be successful even if remote is unreachable. But we can build on this in the future to have an async mechanism to remove the retention lease.
Renamed the method as suggested for now.
soosinha
force-pushed
the
remote_down_stop_repl
branch
from
5767129 to
e72aee2
Compare
Signed-off-by: Sooraj Sinha <[email protected]>
soosinha
force-pushed
the
remote_down_stop_repl
branch
from
e72aee2 to
9530b38
Compare
Signed-off-by: Sooraj Sinha [email protected]
Description
Stop replication should work even when remote cluster is unavailable. Currently, stop replication errors out if remote cluster is unavailable because remove retention lease method throws an exception.
This change catches the error error thrown by remove retention lease method so the stop replication API can go through. Also, added an integ test to verify the same
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.