Open nick-nachos opened 6 years ago
Interesting. I don't have time to work on ConstructR these days, but a PR would be appreciated.
That is great. Would you mind having a look at #168 as well, to see if you agree (in general) with the proposed solution?
Hi, this seems to be related to #167 and its corresponding PR #169, being removeSelf
the equivalent of close
. I'd appreciate some feedback on it :nerd_face:
Both lock and seed-node entries get a TTL after which they expire at the backend. Given this, there is no attempt at the codebase to delete them when it is actually allowed. These cases would be:
While not deleting these entries does not create some catastrophic failure (since they will eventually expire), they do have some annoying side effects:
Couldn't acquire lock, going to GettingNodes
due to the existence of a lock that could have been released, but has just been left to expire (the reason becomes more frequent in the context of #168)My suggestion would be to add two extra methods on the
Coordination
trait:with default implementations that would do nothing (to keep backwards compatibility) and that would attempt to delete lock and seed node entries respectively on a best effort basis. This means that the ConstructrMachine would fire & forget these commands so as not to make the FSM code any more complex. In the best case, firing these commands will achieve the desired results and in the worst one, nothing changes compared to how the FSM works at the moment.
As mentioned at #168 this is one of the improvements my team would need in the context of our work, which of course we will be more than happy to contribute via PR. Your thoughts?