You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Benedict Elliott Smith (Jira)" <ji...@apache.org> on 2024/12/16 21:08:00 UTC
[jira] [Comment Edited] (CASSANDRA-20147) Race in BatchCommitLogService in 4.1+
[ https://issues.apache.org/jira/browse/CASSANDRA-20147?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17906197#comment-17906197 ]
Benedict Elliott Smith edited comment on CASSANDRA-20147 at 12/16/24 9:07 PM:
------------------------------------------------------------------------------
Thanks for the bug report. I think it's possible the problem is slightly different. It looks like we may be overflowing the maximum number of representable Semaphore permits (i.e. we have around 2B permits). I will be honest, I am surprised that these are the semantics of a Semaphore. I would expect you could specify a maximum number of permits and have them saturate. It is also surprising that I've never encountered this edge case before.
It's worth noting this bug appears to have existed since 2013, except for a brief period seemingly 2018 to 2021. It's possible the reversion was a bad port from an older branch where the change to `LockSupport` wasn't made. It might be better to either revert to LockSupport, or a light abstraction around it (i.e. introduce an interface with acquire/release representing a Semaphore with a single saturating permit, and default to LockSupport)
was (Author: benedict):
Thanks for the bug report. I think it's possible the problem is slightly different. It looks like we may be overflowing the maximum number of representable Semaphore permits (i.e. we have around 2B permits). I will be honest, I am surprised that these are the semantics of a Semaphore. I would expect you could specify a maximum number of permits and have them saturate. It is also surprising that I've never encountered this edge case before.
It's worth noting this bug appears to have existed since 2013, except for a brief period seemingly 2018 to 2021. It's also possible this was a bad port from an older branch where the change to `LockSupport` wasn't made. It might be better to either revert to LockSupport, or a light abstraction around it (i.e. introduce an interface with acquire/release representing a Semaphore with a single saturating permit, and default to LockSupport)
> Race in BatchCommitLogService in 4.1+
> -------------------------------------
>
> Key: CASSANDRA-20147
> URL: https://issues.apache.org/jira/browse/CASSANDRA-20147
> Project: Apache Cassandra
> Issue Type: Bug
> Components: Local/Commit Log
> Reporter: Elliott Sims
> Priority: Normal
> Fix For: 4.1.x
>
>
> Saw this crash in production on multiple hosts:
> {code:java}
> ERROR [MutationStage-165] 2024-12-16 04:36:35,914 JVMStabilityInspector.java:68 - Exception in thread Thread[MutationStage-165,5,SharedPool]
> java.lang.RuntimeException: java.lang.Error: Maximum permit count exceeded
> at org.apache.cassandra.net.InboundSink.accept(InboundSink.java:108)
> at org.apache.cassandra.net.InboundSink.accept(InboundSink.java:45)
> at org.apache.cassandra.net.InboundMessageHandler$ProcessMessage.run(InboundMessageHandler.java:430)
> at org.apache.cassandra.concurrent.ExecutionFailure$1.run(ExecutionFailure.java:133)
> at org.apache.cassandra.concurrent.SEPWorker.run(SEPWorker.java:142)
> at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
> at java.base/java.lang.Thread.run(Thread.java:829)
> Caused by: java.lang.Error: Maximum permit count exceeded
> at java.base/java.util.concurrent.Semaphore$Sync.tryReleaseShared(Semaphore.java:198)
> at java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer.releaseShared(AbstractQueuedSynchronizer.java:1382)
> at java.base/java.util.concurrent.Semaphore.release(Semaphore.java:619)
> at org.apache.cassandra.db.commitlog.AbstractCommitLogService.requestExtraSync(AbstractCommitLogService.java:297)
> at org.apache.cassandra.db.commitlog.BatchCommitLogService.maybeWaitForSync(BatchCommitLogService.java:40)
> at org.apache.cassandra.db.commitlog.AbstractCommitLogService.finishWriteFor(AbstractCommitLogService.java:284)
> at org.apache.cassandra.db.commitlog.CommitLog.add(CommitLog.java:330)
> at org.apache.cassandra.db.CassandraKeyspaceWriteHandler.addToCommitLog(CassandraKeyspaceWriteHandler.java:100)
> at org.apache.cassandra.db.CassandraKeyspaceWriteHandler.beginWrite(CassandraKeyspaceWriteHandler.java:54)
> at org.apache.cassandra.db.Keyspace.applyInternal(Keyspace.java:641)
> at org.apache.cassandra.db.Keyspace.applyFuture(Keyspace.java:489)
> at org.apache.cassandra.db.Mutation.applyFuture(Mutation.java:223)
> at org.apache.cassandra.db.MutationVerbHandler.doVerb(MutationVerbHandler.java:63)
> at org.apache.cassandra.net.InboundSink.lambda$new$0(InboundSink.java:78)
> at org.apache.cassandra.net.InboundSink.accept(InboundSink.java:97)
> ... 6 common frames omitted {code}
> Digging through, it looks like in 4.1.0 {{AbstractCommitLogService}} was reworked to use Semaphore instead of LockSupport. From a skim of the docs, {{LockSupport.unpark()}} is safe to run even if the permit's already available where {{Semaphore.release()}} is not.
> I think that means that if more writes arrive during the right part of a flush, it double-decrements `haveWork`. Once it's decremented and starts throwing, I think it never successfully makes it to another acquire/try
> The least-invasive fix is probably to call `haveWork.drainPermits()` in `requestExtraSync()` right before `release(1)` to ensure that `havework.permits` is 0 right before we increment it. That may technically still leave a race, but an almost-impossibly small one rather than a nontrivial one. It might also be possible to catch that specific Exception and run acquire() or (probably better) drainPermits() and recover. That does feel like a weird thing to do for a semaphore exception, but should be OK in this case since it's just being used to sleep the worker loop instead of actual concurrency control.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org