Use pipelines to read group state data, improved error handling - #896
Merged
Conversation
Here we solve two problems: 1. Potential memory issues related to loading all groups data into the memory at once 2. Not enough context about failed data read operation and failing entire operation because one problematic file Previously, we were loading all group data bytes into the memory and iterating over those data to create Memberships and store them in group registry. In case of any error related to reading storage directory or decrypting data, we were failing entire operation without providing enough context about which file caused problems. We decided to refactor group data loading code into go pipelines and refine the information sent through the channels so that we can print error with a precise information about the file (group membership) which could not be loaded.
We no longer return an error from LoadExistingGroup. In case of any problematic situation, we log the error to STDERR.
We start this function as `go groupRegistry.UnregisterStaleGroups()` in beacon.go and we ignore errors there. There is really nothing else we can do than logging an error in case something wrong happened in `UnregisterStaleGroups`. To avoid losing the errors, we now log them in `UnregisterStaleGroups` and this function no longer return any error.
Tests have been refactored to avoid deadlocks - channels are not buffered so we need to read from them asynchronously to do not block the test on ReadAll execution.
File name should go as first, then directory name and callback.
Contributor
|
Testing looks good. I manually corrupted 2 files and they were not loaded. |
dimpar
approved these changes
Jun 30, 2019
dimpar
left a comment
dimpar
left a comment
Contributor
There was a problem hiding this comment.
Looks very nice! I have no comments.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Closes: #657
Here we solve two problems:
Potential memory issues related to loading all groups data into the memory at once
Not enough context about failed data read operation and failing the entire operation because of one problematic file
Previously, we were loading all group data bytes into the memory and iterating over those data to create Memberships and store them in group registry. In case of any error related to reading storage directory or decrypting data, we were failing entire operation without providing enough context about which file caused problems.
We decided to refactor group data loading code into go pipelines and refine the information sent through the channels so that we can print error with precise information about the file (group membership) which could not be loaded.
See: https://blog.golang.org/pipelines
Proof
File corrupted:
All files correct: