dcapwell commented on code in PR #2144:
URL: https://github.com/apache/cassandra/pull/2144#discussion_r1129771327
##########
src/java/org/apache/cassandra/service/accord/AccordStateCache.java:
##########
@@ -245,9 +245,8 @@ private <K, V> void updateSize(Node<K, V> node,
ToLongFunction<V> estimator)
// before it's mutation is applied, out of date info will be loaded
private boolean canEvict(Node<?, ?> node)
{
- return node.references == 0 &&
- node.isLoaded() &&
- !hasActiveAsyncResult(saveResults, node.key());
+ Invariants.checkState(node.references == 0);
+ return node.state() == FAILED || !hasActiveAsyncResult(saveResults,
node.key());
Review Comment:
this change wasn't replicated to
`org.apache.cassandra.service.accord.AccordStateCache.Instance#checkCanEvict`.
This is only used in
`org.apache.cassandra.service.accord.async.AsyncOperationTest#assertCanEvict`
and was there to have a more human readable understanding of why we think we
can not evict...
I am ok dropping
`org.apache.cassandra.service.accord.AccordStateCache.Instance#checkCanEvict`
as its just for tests and causes duplication of logic; but does make bugs
harder to track down when tests do finally fail...
--
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]
