Simplify some code paths further and use lock/defer unlock in more places #3818
Conversation
This file contains 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
Codecov Report
@@ Coverage Diff @@
## linasn/bootstrap-after-fileops-wait #3818 +/- ##
=====================================================================
Coverage ? 56.8%
=====================================================================
Files ? 552
Lines ? 63100
Branches ? 0
=====================================================================
Hits ? 35897
Misses ? 24004
Partials ? 3199
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
soundvibe
added a commit
that referenced
this pull request
Oct 6, 2021
* Set `Bootstrapping` state when waiting for fileOps to be disabled completed, because it can take a while. * Release database lock before enqueueing bootstrap because it could take a while. * Rollback bootstrapping state change. * disable/enable fileOps when adding a new namespace. * removed comments added for testing. * fixed linter issues. * fixed potential issue when competing bootstraps could be started and `onCompleteFn` might be invoked for the second bootstrap while another bootstrap is still in progress. This is bad because bootstrap should work when fileOps are disabled and `onCompleteFn` might be enabling fileOps. * fixed typo in comment. * implemented similar enqueue bootstrap logics to `assignShardSet` as well. * Disable both file ops first and then wait for them to stop (should reduce waiting times) * New namespace adds can now be enqueued and be non-blocking. If db was not yet bootstrapped and new shardSet is assigned, do this immediately (no need to enqueue). New test for add new namespace using enqueue. * small cleanup * reverse commit * We can enqueue bootstrap with lock to avoid potential race now because enqueueing and waiting is fully async. * unlock right after `namespaceDeltaWithLock()` * Simplify callback logic by having the mediator own the lifecycle of the callback functions (#3810) * Simplify callback logic by having the mediator own the lifecycle of the callback functions * Remove no longer required BootstrapAsyncResult * Remove unused require.NoError * Fix lint * Added test when bootstrap is enqueued in case new namespaces are added. Fixed possible race in `context.StartSampledTraceSpan()`. Use zap error logging where needed. Make sure fileOps are disabled/enabled only when really needed (bootstraps > 0 and mediator is opened), otherwise just use simpler logics. * Simplify some code paths further and use lock/defer unlock in more places (#3818) * Simplify some code paths further and use lock/defer unlock in more places * Fix test * More test coverage. * Fixed possible race in unit test assertion. Co-authored-by: Rob Skillington <rob.skillington@gmail.com>
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.
What this PR does / why we need it:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: