dcapwell commented on code in PR #38:
URL: https://github.com/apache/cassandra-accord/pull/38#discussion_r1177151414
##########
accord-core/src/main/java/accord/local/CommandStore.java:
##########
@@ -43,7 +48,88 @@ CommandStore create(int id,
RangesForEpochHolder rangesForEpoch);
}
+ @VisibleForTesting
+ class Unsafe
Review Comment:
> Should this be in the InMemoryCommandStore
Not really... If we are working with `CommandStore` we need the API to be
present there.
`Unsafe` has 2 things going on:
1) holds the reference to the `ThreadLocal` pointing to the `current`
`CommandStore`.
2) operations for tests to mutate the `ThreadLocal` that maybe "unsafe"
I tried to limit the scope of the `ThreadLocal` reference to make access
limited to only the "approved" methods, that was an attempt to limit bugs.
Now, for the Unsafe operations (num 2), those are used by more than
`InMemoryCommandStore`; current usages are
1) `accord.topology.TopologyManager#awaitEpoch` - Need to know if running
in a `CommandStore` or not, so we don't return a `AsyncChain` using the wrong
store
2) `accord.local.Node#checkStore` - makes sure that the selected
`CommandStore` is the current one; this has been critical to make sure that we
don't do the wrong thing.
3) `Synchronized` and `DelayedCommandStores` - since no threading is done,
this is to support the interleaving of tasks, which may be running in different
`CommandStore`s.
For 1/2 we just need access to `current` that *does not* fail, I left this
in `Unsafe` to at least denote that you must be careful, and to try to avoid
the wrong code paths calling it... this "could" be moved outside of `Unsafe`,
but then I would likely rename to `CommandStore.unsafeMaybeCurrent` or
something (just trying to denote not to call it)
for 3, I "could" try to limit this to `InMemoryCommandStore`, but I would
need to expose `register` and `remove`; that or have to make the `ThreadLocal`
public, which I was trying to avoid
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]