Skip to content
This repository has been archived by the owner on Jul 19, 2020. It is now read-only.

ensure consensus changes are only applied as a whole #24

Open
GlenDC opened this issue Aug 28, 2018 · 1 comment
Open

ensure consensus changes are only applied as a whole #24

GlenDC opened this issue Aug 28, 2018 · 1 comment

Comments

@GlenDC
Copy link
Contributor

GlenDC commented Aug 28, 2018

Currently we return with an error as soon as an error happens, even if it's in the middle of a consensus change that is already partially applied (applied as in the Redis DB has already been modified with the partial content of the consensus change in question).

A better approach would be where we keep track of all the changes we make, if do-able watch the keys while getting them, and only at the end, apply all changes at once, using the ACID mechanics Redis provides.

This would mean that we need to have an interface which:

  • for modification (delete/write) commands keeps track of that change purely in-memory;
  • for get commands checks first if we have an in-memory state for the value in question, if so return, if not get it actually from the redis-db, and again if possible and do-able watch it as well;

The watching is nice to have, but not as important, given that we assume we are the only writers to the RedisDB. It does however protect against multiple writers should that situation ever occur. However, as stated, we assume we are the only writers so this can be ignored for now I think, especially given that I do not think we can do it without making a new connection just for the watching (that and the commit in the end) which seems kinda expensive.

@GlenDC GlenDC added this to the v0.2.0 milestone Aug 28, 2018
@GlenDC GlenDC self-assigned this Aug 28, 2018
@GlenDC GlenDC modified the milestones: v0.2.0, v0.2.1 Sep 7, 2018
@GlenDC GlenDC modified the milestones: v0.2.1, backlog Sep 17, 2018
@GlenDC
Copy link
Contributor Author

GlenDC commented Sep 17, 2018

This is definitely a feature that we can use, but it is not critically important. This issue would prevent corrupt databases that are triggered by ourselves, due to a panic. Such a panic always indicates a bug that most likely has to be fixed to unblock the user. As the user will have to wait "long" to get such a bug fixed, it will be not a problem for the user to lose some minutes more in order to repopulate the redis db from scratch.

It will however be nice, as it means that a panic does not corrupt a db. I also have the feeling that for the populating-from-boltdb case it will make it a lot faster, given that we do not constantly have to read/write values, and will do it in batches (or perhaps as a whole even? If not as a whole, the rollback feature it promises does become a lot trickier) instead.

Even so, as long as there are no bugs in the redis db populating process, and given the testing we do it seems unlikely, it is not a feature the user will really feel. And definitely not the average user, which was the entire motivational target for this project. Therefore this issue can wait for when the project is feature-complete.

For now issues such as #32 are much more important IMHO.

@GlenDC GlenDC removed their assignment Dec 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant