Bug fix 3.5/min replication factor #9524
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Improve collection time of IResearchQueryOptimizationTest * Added a minReplicationFactor field in Collections. It is not possible to modify it yet and noone cares for it * Added some assertion son minReplicationFactor * Transaction API will now reject writes as soon as minimal replication factor is NOT fulfilled * added minReplicationFactor to the user interface, preparation for the collection api changes * added minReplicationFactor to VocBaseCollection, RestReplicationHandler, RestCollectionHandler, ClusterMethods, ClusterInfo and ClusterCollectionCreationInfo * added minReplicationFactor usage to tests * TODO TEMOPORARY COMMIT FOR TESTING PLEASE REVERT ME * minReplicationFactor now able to change via collection properties route * fixed wrongly assert * added minReplicationFactor to the graph management ui * added minReplicationFactor to the gharial api * Fixed off-by-one error in minReplicationFactor. We actually enforced one more. * adjusted description of minReplicationFactor * FollowerInfo Refactoring * added gharial api graph creation tests with minimal replication factor * proper cleanup of shell collection tests, removed lots of duplicate code, preparation for some new tests * added collection create tests using invalid/valid names, replicationFactor and minReplicationFactor * Debug logging * MORE Debug logging * Included replication fast lane * Use correct minreplicationfactor * modified debug logging * Fixed compileissues * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * Revert "MORE Debug logging" This reverts commit dab5af2. * Revert "MORE Debug logging" This reverts commit 6134b66. * Revert "MORE Debug logging" This reverts commit 80160bd. * Revert "MORE Debug logging" This reverts commit 06aabcd. * Removed debug output * Added replication fast lane. Also refactored the commands as i cannot take it any more... * Put some requests of RocksDBReplication onto CATCHUP Lane. * Put some requests of MMFilesReplication onto CATCHUP Lane. * Adjusted Fast and MED lane usage in Supervised scheduler * Added changelog entry * Added new features entry * A new leader will now keep old followers in case of failover * Update arangod/Cluster/ClusterCollectionCreationInfo.cpp Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Fixed JSLINT * Unified lane handling of replication handlers * Sorry forgotten in last commit * replaced strings with static strings * more use of static strings * optimized min repl description in the ui * decr initial loop variable * clean up of the createWithId test * more use of static strings * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/collectionsView.js Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Added some comments on condition, renamed variable as suggested in review * Added check for min replicationFactor to be non-zero * Added assertion * Added function to modify min and max replication factor in one go * added missing semicolon * rm log devel * Added a second information to follower info that can keep track of followers that have been in sync before a failover has taken place * Maintenance reports previous version now to follower info. instead of lying by itself. The Follower Info now gets a failover save mode to report insync followers * check replFactor against nr dbservers * Add lie reporting in CURRENT * Reverted most of my recent commits about Failover situation. The intended plan simply does not work out * move replication checks from logical collection to rest collection handler * added more replication tests * Include assert only if we are not in gtest * jslint * set min repl factor to zero if satellite collection * check replication attributes in v8 collection * Initial commit, old plan, does not yet work * fixed ires tests * Included FailoverCandidates key. Not fully implemented * fixed wrong assert * unified in sync follower reporting * fixed compiler errors * Cleanup locking, and fixed potential deadlocks * Comments about locking order in FollowerInfo. * properly check uint * Keep old leader as potential failover candidate * Transaction methods now use followerInfo to check if the leader can write, this might have the sideeffect that 'failoverCandidates' are updated * Let agency check failoverCandidates if possible * Initialize member variables * Use unified follower reporting in DBServerAgencySync * Removed obsolete variable, collecting it somewhere else * repl factor attr check * Reimplemented previous followers, second attempt now. PhaseOne and PhaseTwo can now synchronize on current. * Fixed assertion, forgot an off-by-one * adjusted test to be more preciese now * Fixed failove candidates list * Disable write on dropping too many followers * Allow to run updateFailoerCandidates multiple times with same leader. * Final fixes, resilience tests now green, crossing fingers for jenkins * Fixed race on atomics comparison * Fixed invalid number type * added nullptr handling * added nullptr handling * Removed invalid assert * Make takeover of leadership an atomic operation * Update tests/js/common/shell/shell-cluster-collection.js Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Review fixes * Fixed creation code to use takeoverLeadership * Update arangod/Cluster/FollowerInfo.h Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Applied review fixes * There is no timeout * Moved AQL + Pregel to INTERNAL_AQL lane, which is medium priority, to avoid deadlocks with Sync replication * More review fixes * Use difference if you want to compare two vectors... * Use std::string ... * Now check if we are in recovery mode * Added documentation for minReplicationFactor * Added readme update as well in documenation
jsteemann
suggested changes
Jul 19, 2019
Documentation/Books/Manual/DataModeling/Collections/DatabaseMethods.md
Outdated
Show resolved
Hide resolved
|
|
||
| // Check if we are allowed to pull from a queue with the given index | ||
| // This is used to give priority to "FAST" and "MED" lanes accordingly. | ||
| inline bool canPullFromQueue(uint64_t queueIdx) const; |
There was a problem hiding this comment.
the inline doesn't make sense here.
js/apps/system/_admin/aardvark/APP/frontend/js/views/collectionsView.js
Outdated
Show resolved
Hide resolved
js/apps/system/_admin/aardvark/APP/frontend/js/views/collectionsView.js
Outdated
Show resolved
Hide resolved
js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js
Outdated
Show resolved
Hide resolved
js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js
Outdated
Show resolved
Hide resolved
…nsView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
…nsView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
…thods.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
…gementView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
…gementView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
…rangodb/ArangoDB into bug-fix-3.5/minReplicationFactor
Co-Authored-By: Jan <jsteemann@users.noreply.github.com>
|
Adopted all changes thanks @jsteemann |
jsteemann
suggested changes
Jul 22, 2019
| // There should be no new workers. | ||
| // Current workers could be stuck on the condition variable. | ||
| // Let's wake them up now. | ||
| _currentCounterCondition.notify_all(); |
There was a problem hiding this comment.
I think we should hold the mutex here when we call notify_all():
std::unique_lock<std::mutex> guard(_currentCounterLock);
Simran-B
requested changes
Jul 22, 2019
There was a problem hiding this comment.
Only DocuBlocks need to be added in arangodb/arangodb, the rest of content needs to be comitted to arangodb/docs. There is an open PR here: arangodb/docs#108
One of the descriptions deviates from the text in this PR, please check if any changes to the docs PR are necessary.
| on different DBServers. | ||
| on different DBServers. Has to be in the range of 1-10 *(Cluster only)* | ||
|
|
||
| * *minReplicationFactor* : determines the number of minimal shard copies kept on |
There was a problem hiding this comment.
should probably not be indented
Documentation/Books/Manual/DataModeling/Collections/DatabaseMethods.md
Outdated
Show resolved
Hide resolved
jsteemann
approved these changes
Jul 22, 2019
Simran-B
approved these changes
Jul 22, 2019
KVS85
approved these changes
Jul 22, 2019
mchacki
added a commit
that referenced
this pull request
Jul 23, 2019
* Cherry-pick minReplicationFactor * Bug fix/failover with min replication factor (#9486) * Improve collection time of IResearchQueryOptimizationTest * Added a minReplicationFactor field in Collections. It is not possible to modify it yet and noone cares for it * Added some assertion son minReplicationFactor * Transaction API will now reject writes as soon as minimal replication factor is NOT fulfilled * added minReplicationFactor to the user interface, preparation for the collection api changes * added minReplicationFactor to VocBaseCollection, RestReplicationHandler, RestCollectionHandler, ClusterMethods, ClusterInfo and ClusterCollectionCreationInfo * added minReplicationFactor usage to tests * TODO TEMOPORARY COMMIT FOR TESTING PLEASE REVERT ME * minReplicationFactor now able to change via collection properties route * fixed wrongly assert * added minReplicationFactor to the graph management ui * added minReplicationFactor to the gharial api * Fixed off-by-one error in minReplicationFactor. We actually enforced one more. * adjusted description of minReplicationFactor * FollowerInfo Refactoring * added gharial api graph creation tests with minimal replication factor * proper cleanup of shell collection tests, removed lots of duplicate code, preparation for some new tests * added collection create tests using invalid/valid names, replicationFactor and minReplicationFactor * Debug logging * MORE Debug logging * Included replication fast lane * Use correct minreplicationfactor * modified debug logging * Fixed compileissues * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * Revert "MORE Debug logging" This reverts commit dab5af2. * Revert "MORE Debug logging" This reverts commit 6134b66. * Revert "MORE Debug logging" This reverts commit 80160bd. * Revert "MORE Debug logging" This reverts commit 06aabcd. * Removed debug output * Added replication fast lane. Also refactored the commands as i cannot take it any more... * Put some requests of RocksDBReplication onto CATCHUP Lane. * Put some requests of MMFilesReplication onto CATCHUP Lane. * Adjusted Fast and MED lane usage in Supervised scheduler * Added changelog entry * Added new features entry * A new leader will now keep old followers in case of failover * Update arangod/Cluster/ClusterCollectionCreationInfo.cpp Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Fixed JSLINT * Unified lane handling of replication handlers * Sorry forgotten in last commit * replaced strings with static strings * more use of static strings * optimized min repl description in the ui * decr initial loop variable * clean up of the createWithId test * more use of static strings * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/collectionsView.js Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Added some comments on condition, renamed variable as suggested in review * Added check for min replicationFactor to be non-zero * Added assertion * Added function to modify min and max replication factor in one go * added missing semicolon * rm log devel * Added a second information to follower info that can keep track of followers that have been in sync before a failover has taken place * Maintenance reports previous version now to follower info. instead of lying by itself. The Follower Info now gets a failover save mode to report insync followers * check replFactor against nr dbservers * Add lie reporting in CURRENT * Reverted most of my recent commits about Failover situation. The intended plan simply does not work out * move replication checks from logical collection to rest collection handler * added more replication tests * Include assert only if we are not in gtest * jslint * set min repl factor to zero if satellite collection * check replication attributes in v8 collection * Initial commit, old plan, does not yet work * fixed ires tests * Included FailoverCandidates key. Not fully implemented * fixed wrong assert * unified in sync follower reporting * fixed compiler errors * Cleanup locking, and fixed potential deadlocks * Comments about locking order in FollowerInfo. * properly check uint * Keep old leader as potential failover candidate * Transaction methods now use followerInfo to check if the leader can write, this might have the sideeffect that 'failoverCandidates' are updated * Let agency check failoverCandidates if possible * Initialize member variables * Use unified follower reporting in DBServerAgencySync * Removed obsolete variable, collecting it somewhere else * repl factor attr check * Reimplemented previous followers, second attempt now. PhaseOne and PhaseTwo can now synchronize on current. * Fixed assertion, forgot an off-by-one * adjusted test to be more preciese now * Fixed failove candidates list * Disable write on dropping too many followers * Allow to run updateFailoerCandidates multiple times with same leader. * Final fixes, resilience tests now green, crossing fingers for jenkins * Fixed race on atomics comparison * Fixed invalid number type * added nullptr handling * added nullptr handling * Removed invalid assert * Make takeover of leadership an atomic operation * Update tests/js/common/shell/shell-cluster-collection.js Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Review fixes * Fixed creation code to use takeoverLeadership * Update arangod/Cluster/FollowerInfo.h Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Applied review fixes * There is no timeout * Moved AQL + Pregel to INTERNAL_AQL lane, which is medium priority, to avoid deadlocks with Sync replication * More review fixes * Use difference if you want to compare two vectors... * Use std::string ... * Now check if we are in recovery mode * Added documentation for minReplicationFactor * Added readme update as well in documenation * Removed merge conflict leftovers 0o, i should not trust the IDE * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/collectionsView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/collectionsView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update Documentation/Books/Manual/Architecture/Replication/README.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update CHANGELOG Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update Documentation/Books/Manual/DataModeling/Collections/DatabaseMethods.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update Documentation/Books/Manual/ReleaseNotes/NewFeatures35.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update Documentation/DocuBlocks/Rest/Collections/1_structs.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update Documentation/DocuBlocks/Rest/Graph/1_structs.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Apply suggestions from code review Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Adepted review requests, thanks for finding! * Removed unnecessary const * Apply suggestions from code review Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Moved initilization of variable more downwards * Apply lock before notify_all() * Remove documentation except DocuBlocks, covered by PR in docs repo * Remove accidental indent
mchacki
added a commit
that referenced
this pull request
Jul 23, 2019
* Bug fix 3.5/min replication factor (#9524) * Cherry-pick minReplicationFactor * Bug fix/failover with min replication factor (#9486) * Improve collection time of IResearchQueryOptimizationTest * Added a minReplicationFactor field in Collections. It is not possible to modify it yet and noone cares for it * Added some assertion son minReplicationFactor * Transaction API will now reject writes as soon as minimal replication factor is NOT fulfilled * added minReplicationFactor to the user interface, preparation for the collection api changes * added minReplicationFactor to VocBaseCollection, RestReplicationHandler, RestCollectionHandler, ClusterMethods, ClusterInfo and ClusterCollectionCreationInfo * added minReplicationFactor usage to tests * TODO TEMOPORARY COMMIT FOR TESTING PLEASE REVERT ME * minReplicationFactor now able to change via collection properties route * fixed wrongly assert * added minReplicationFactor to the graph management ui * added minReplicationFactor to the gharial api * Fixed off-by-one error in minReplicationFactor. We actually enforced one more. * adjusted description of minReplicationFactor * FollowerInfo Refactoring * added gharial api graph creation tests with minimal replication factor * proper cleanup of shell collection tests, removed lots of duplicate code, preparation for some new tests * added collection create tests using invalid/valid names, replicationFactor and minReplicationFactor * Debug logging * MORE Debug logging * Included replication fast lane * Use correct minreplicationfactor * modified debug logging * Fixed compileissues * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * MORE Debug logging * Revert "MORE Debug logging" This reverts commit dab5af2. * Revert "MORE Debug logging" This reverts commit 6134b66. * Revert "MORE Debug logging" This reverts commit 80160bd. * Revert "MORE Debug logging" This reverts commit 06aabcd. * Removed debug output * Added replication fast lane. Also refactored the commands as i cannot take it any more... * Put some requests of RocksDBReplication onto CATCHUP Lane. * Put some requests of MMFilesReplication onto CATCHUP Lane. * Adjusted Fast and MED lane usage in Supervised scheduler * Added changelog entry * Added new features entry * A new leader will now keep old followers in case of failover * Update arangod/Cluster/ClusterCollectionCreationInfo.cpp Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Fixed JSLINT * Unified lane handling of replication handlers * Sorry forgotten in last commit * replaced strings with static strings * more use of static strings * optimized min repl description in the ui * decr initial loop variable * clean up of the createWithId test * more use of static strings * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/collectionsView.js Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Added some comments on condition, renamed variable as suggested in review * Added check for min replicationFactor to be non-zero * Added assertion * Added function to modify min and max replication factor in one go * added missing semicolon * rm log devel * Added a second information to follower info that can keep track of followers that have been in sync before a failover has taken place * Maintenance reports previous version now to follower info. instead of lying by itself. The Follower Info now gets a failover save mode to report insync followers * check replFactor against nr dbservers * Add lie reporting in CURRENT * Reverted most of my recent commits about Failover situation. The intended plan simply does not work out * move replication checks from logical collection to rest collection handler * added more replication tests * Include assert only if we are not in gtest * jslint * set min repl factor to zero if satellite collection * check replication attributes in v8 collection * Initial commit, old plan, does not yet work * fixed ires tests * Included FailoverCandidates key. Not fully implemented * fixed wrong assert * unified in sync follower reporting * fixed compiler errors * Cleanup locking, and fixed potential deadlocks * Comments about locking order in FollowerInfo. * properly check uint * Keep old leader as potential failover candidate * Transaction methods now use followerInfo to check if the leader can write, this might have the sideeffect that 'failoverCandidates' are updated * Let agency check failoverCandidates if possible * Initialize member variables * Use unified follower reporting in DBServerAgencySync * Removed obsolete variable, collecting it somewhere else * repl factor attr check * Reimplemented previous followers, second attempt now. PhaseOne and PhaseTwo can now synchronize on current. * Fixed assertion, forgot an off-by-one * adjusted test to be more preciese now * Fixed failove candidates list * Disable write on dropping too many followers * Allow to run updateFailoerCandidates multiple times with same leader. * Final fixes, resilience tests now green, crossing fingers for jenkins * Fixed race on atomics comparison * Fixed invalid number type * added nullptr handling * added nullptr handling * Removed invalid assert * Make takeover of leadership an atomic operation * Update tests/js/common/shell/shell-cluster-collection.js Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Review fixes * Fixed creation code to use takeoverLeadership * Update arangod/Cluster/FollowerInfo.h Co-Authored-By: Tobias Gödderz <tobias@arangodb.com> * Applied review fixes * There is no timeout * Moved AQL + Pregel to INTERNAL_AQL lane, which is medium priority, to avoid deadlocks with Sync replication * More review fixes * Use difference if you want to compare two vectors... * Use std::string ... * Now check if we are in recovery mode * Added documentation for minReplicationFactor * Added readme update as well in documenation * Removed merge conflict leftovers 0o, i should not trust the IDE * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/collectionsView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/collectionsView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update Documentation/Books/Manual/Architecture/Replication/README.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update CHANGELOG Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update Documentation/Books/Manual/DataModeling/Collections/DatabaseMethods.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update Documentation/Books/Manual/ReleaseNotes/NewFeatures35.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update Documentation/DocuBlocks/Rest/Collections/1_structs.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update js/apps/system/_admin/aardvark/APP/frontend/js/views/graphManagementView.js Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Update Documentation/DocuBlocks/Rest/Graph/1_structs.md Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Apply suggestions from code review Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Adepted review requests, thanks for finding! * Removed unnecessary const * Apply suggestions from code review Co-Authored-By: Jan <jsteemann@users.noreply.github.com> * Moved initilization of variable more downwards * Apply lock before notify_all() * Remove documentation except DocuBlocks, covered by PR in docs repo * Remove accidental indent * Removed leftover merge conflict in documentation block
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of minReplicationFactor devel => 3.5
Has been reviewed in devel intensively. No changes had to be applied.
Jenkins:
http://jenkins01.arangodb.biz:8080/view/PR/job/arangodb-matrix-pr/5325/