This repository has been archived by the owner on Oct 4, 2022. It is now read-only.
Refactor state management outside the overlord. #18
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
This will ease future usage of a central DB for state between an array of overlords.
n3vermind
reviewed
Sep 10, 2018
n3vermind
reviewed
Sep 10, 2018
n3vermind
reviewed
Sep 10, 2018
src/go/overlord/statemanager.go
Outdated
| } | ||
|
|
||
| // LocalStateManager handles state through a local time-expiring cache. | ||
| type LocalStateManager struct { |
There was a problem hiding this comment.
You could move this to a separate package (say, 'state'), and name this Local. Then calling this would be state.NewLocal(), and in future also state.NewFoobar where Foobar is some form of a distributed state manager.
There was a problem hiding this comment.
I think this is a good idea. One drawback though is that mappedInterest is now caught between the 2 packages (it's defined in the overlord, I was using it in state). Any suggestions on how to break this?
There was a problem hiding this comment.
Ok, I've moved everything into the state package :-) Let me know what you think
Move state interface where it's used. [Note this does not compile yet, followup CL on interests comng up]
Move mappedInterest to the state package as an exported struct.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This will ease future usage of a central DB for state between an array of overlords.
It uses a local cache through the same library and general set up we have in the Minion, making it a bit easier to test parts of the Overlord.
Note that in this refactoring I pulled out the chunk-merging code away from the overlord and into the implementation of the state manager, which might - or quite likely might not - be the right thing to do.