josh-mckenzie commented on a change in pull request #1213:
URL: https://github.com/apache/cassandra/pull/1213#discussion_r724246464
##########
File path: src/java/org/apache/cassandra/schema/PartitionDenylist.java
##########
@@ -161,36 +159,35 @@ private boolean readAllHasSufficientNodes()
/** Helper method as we need to both build cache on initial init but also
on reload of cache contents and params */
private LoadingCache<TableId, DenylistEntry> buildEmptyCache()
{
- return CacheBuilder.newBuilder()
-
.refreshAfterWrite(DatabaseDescriptor.getDenylistRefreshSeconds(),
TimeUnit.SECONDS)
- .build(new CacheLoader<TableId, DenylistEntry>()
+ return Caffeine.newBuilder()
+
.refreshAfterWrite(DatabaseDescriptor.getDenylistRefreshSeconds(),
TimeUnit.SECONDS)
+ .executor(executor)
+ .build(new CacheLoader<TableId, DenylistEntry>()
+ {
+ @Override
+ public DenylistEntry load(final TableId tid) throws
Exception
{
- @Override
- public DenylistEntry load(final TableId tid)
throws Exception
- {
- return getDenylistForTable(tid);
- }
-
- @Override
- public ListenableFuture<DenylistEntry>
reload(final TableId tid, final DenylistEntry oldValue)
- {
- ListenableFutureTask<DenylistEntry> task =
ListenableFutureTask.create(new Callable<DenylistEntry>()
- {
- @Override
- public DenylistEntry call()
- {
- final DenylistEntry newEntry =
getDenylistForTable(tid);
- if (newEntry != null)
- return newEntry;
- if (oldValue != null)
- return oldValue;
- return new DenylistEntry();
- }
- });
- executor.execute(task);
- return task;
- }
- });
+ return getDenylistForTableFromCQL(tid);
+ }
+
+ @Override
+ public CompletableFuture<DenylistEntry>
asyncReload(final TableId tid, final DenylistEntry oldValue, Executor executor)
+ {
+ return CompletableFuture.supplyAsync(() -> {
+ /* We accept DenylistEntry data in the
following precedence:
+ 1) new data pulled from CQL if we got
it correctly,
+ 2) the old value from the cache if
there was some error querying,
+ 3) an empty record otherwise.
+ */
+ final DenylistEntry newEntry =
getDenylistForTableFromCQL(tid);
+ if (newEntry != null)
+ return newEntry;
+ if (oldValue != null)
+ return oldValue;
+ return new DenylistEntry();
+ }, executor);
+ }
Review comment:
The thing that wasn't clear to me was the execution context (i.e. thread
pool / executor etc) that sync reload calls occurred on. So long as that's on
the provided executor for the builder, strong :+1: to tidying this up.
--
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]
