-
Couldn't load subscription status.
- Fork 76
Adding Index Settings validation before starting replication #461
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
Conversation
Signed-off-by: Gaurav Bafna <gbbafna@amazon.com>
Codecov Report
@@ Coverage Diff @@
## main #461 +/- ##
============================================
- Coverage 74.52% 72.54% -1.98%
+ Complexity 1002 985 -17
============================================
Files 141 141
Lines 4565 4579 +14
Branches 504 506 +2
============================================
- Hits 3402 3322 -80
- Misses 847 937 +90
- Partials 316 320 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| .build() | ||
|
|
||
| assertThatThrownBy { | ||
| followerClient.startReplication(StartReplicationRequest("source", leaderIndexName, followerIndexName, settings = settings)) |
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.
Can we validated status code as well?
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.
done .
| overriddenSettings: Settings, | ||
| metadataCreateIndexService: MetadataCreateIndexService | ||
| ) { | ||
| val settingsList = arrayOf(leaderSettings, overriddenSettings) |
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.
Please validate to see if the behavior is as expected, due to open bug on replication side: #298?
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 have fixed this bug in this PR as well. Now we are retrieving the default settings as well .
Signed-off-by: Gaurav Bafna <gbbafna@amazon.com>
6fb72bf to
afb9fbb
Compare
* Adding Index Settings validation before starting replication Signed-off-by: Gaurav Bafna <gbbafna@amazon.com> * Retrieving default index settings before starting replication Signed-off-by: Gaurav Bafna <gbbafna@amazon.com> (cherry picked from commit 93b43f4)
…470) * Adding Index Settings validation before starting replication Signed-off-by: Gaurav Bafna <gbbafna@amazon.com> * Retrieving default index settings before starting replication Signed-off-by: Gaurav Bafna <gbbafna@amazon.com> (cherry picked from commit 93b43f4) Co-authored-by: Gaurav Bafna <gbbafna@amazon.com>
Signed-off-by: Gaurav Bafna gbbafna@amazon.com
Description
Before starting the replication , we need to validate for correctness of index settings. This would throw validation exception before starting replication itself.
If not done, the snapshot restore phase of replication will fail asynchronously and customer would need to stop and start the replication again without error message being communicated .
Issues Resolved
#460
#298
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.