belliottsmith commented on code in PR #38:
URL: https://github.com/apache/cassandra-accord/pull/38#discussion_r1178847787
##########
accord-core/src/main/java/accord/impl/InMemoryCommandStore.java:
##########
@@ -837,44 +839,43 @@ public void shutdown() {}
public static class SingleThread extends InMemoryCommandStore
{
- private final AtomicReference<Thread> expectedThread = new
AtomicReference<>();
+ private Thread thread; // when run in the executor this will be
non-null, null implies not running in this store
private final ExecutorService executor;
- public SingleThread(int id, NodeTimeService time, Agent agent,
DataStore store, ProgressLog.Factory progressLogFactory, RangesForEpochHolder
rangesForEpochHolder)
+ private SingleThread(int id, NodeTimeService time, Agent agent,
DataStore store, ProgressLog.Factory progressLogFactory, RangesForEpochHolder
rangesForEpochHolder)
{
- this(id, time, agent, store, progressLogFactory,
rangesForEpochHolder, Executors.newSingleThreadExecutor(r -> {
+ super(id, time, agent, store, progressLogFactory,
rangesForEpochHolder);
+ this.executor = Executors.newSingleThreadExecutor(r -> {
Thread thread = new Thread(r);
thread.setName(CommandStore.class.getSimpleName() + '[' +
time.id() + ']');
return thread;
- }));
- }
-
- private SingleThread(int id, NodeTimeService time, Agent agent,
DataStore store, ProgressLog.Factory progressLogFactory, RangesForEpochHolder
rangesForEpochHolder, ExecutorService executor)
- {
- super(id, time, agent, store, progressLogFactory,
rangesForEpochHolder);
- this.executor = executor;
+ });
+ executor.execute(() -> thread = Thread.currentThread());
}
- public static CommandStore.Factory factory(ExecutorService executor)
+ public static SingleThread create(int id, NodeTimeService time, Agent
agent, DataStore store, ProgressLog.Factory progressLogFactory,
RangesForEpochHolder rangesForEpochHolder)
Review Comment:
Minor refactor suggestion:
```
interface Constructor<ST extends SingleThread>
{
ST construct(int id, NodeTimeService time, Agent agent,
DataStore store, ProgressLog.Factory progressLogFactory, RangesForEpochHolder
rangesForEpochHolder);
}
public static SingleThread create(int id, NodeTimeService time,
Agent agent, DataStore store, ProgressLog.Factory progressLogFactory,
RangesForEpochHolder rangesForEpochHolder)
{
return create(id, time, agent, store, progressLogFactory,
rangesForEpochHolder, SingleThread::new);
}
protected static <ST extends SingleThread> ST create(int id,
NodeTimeService time, Agent agent, DataStore store, ProgressLog.Factory
progressLogFactory, RangesForEpochHolder rangesForEpochHolder, Constructor<ST>
constructor)
{
ST st = constructor.construct(id, time, agent, store,
progressLogFactory, rangesForEpochHolder);
st.execute(() -> st.thread = Thread.currentThread());
st.execute(() -> CommandStore.register(st));
return st;
}
```
and then use the same for `Debug` (this also requires making `thread`
package-private, but it is probably overall cleaner this way)
(Though I would personally opt for just doing this in the constructor, since
it is safe, I'm fine with this approach if we make it clear how extending
classes should re-use the factory method)
--
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]
