dcapwell commented on code in PR #38:
URL: https://github.com/apache/cassandra-accord/pull/38#discussion_r1179441719
##########
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:
reading more on JMM it does seem that grouping `final` fields *before* this
"leak" is safe from the final field point of view (scheduling on the thread
before the `final` fields may not see them set it seems) and may not be safe
for non-final fields... in our case those are all local thread processing, so
would be fine...
Given this, I can revert back to constructor and document this assumption.
--
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]
