[ https://issues.apache.org/jira/browse/CASSANDRA-16072?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17186401#comment-17186401 ]
Benjamin Lerer edited comment on CASSANDRA-16072 at 8/28/20, 9:14 AM: ---------------------------------------------------------------------- I might be missing something but {{HintsBuffer.allocateBytes}} logic seems wrong. {code} private int allocateBytes(int totalSize) { int prev = position.getAndAdd(totalSize); if (prev == CLOSED) // the slab has been 'closed' return CLOSED; if ((prev + totalSize) > slab.capacity()) { position.set(CLOSED); // mark the slab as no longer allocating if we've exceeded its capacity return CLOSED; } return prev; } {code} As the following scenario will not produce the expected out come: # Thread A calls {{allocateBytes}} and exit with the return value {{CLOSED}} and {{position == CLOSED}} # Thead B calls {{allocateBytes}} with {{totalSize == size_B}} and exit with the return value {{CLOSED}} and {{position == size_B -1}} as {{CLOSED == -1}} in practice. # Thread C calls {{allocateBytes}} with {{totalSize == size_C}} and exit with the return value {{size_B - 1}} and and {{position == size_B + size_C -1}} was (Author: blerer): I might be missing something but {{HintsBuffer.allocateBytes}} logic seems wrong. {code} private int allocateBytes(int totalSize) { int prev = position.getAndAdd(totalSize); if (prev == CLOSED) // the slab has been 'closed' return CLOSED; if ((prev + totalSize) > slab.capacity()) { position.set(CLOSED); // mark the slab as no longer allocating if we've exceeded its capacity return CLOSED; } return prev; } {code} As the following scenario will not produce the expected out come: # Thread A calls {{allocateBytes}} and exit with the return value {{CLOSED}} and {{position == CLOSED}} # Thead B calls {{allocateBytes}} with {{totalSize == size_B}} and exit with the return value {{CLOSED}} and and {{position == size_B -1}} as {{CLOSED == -1}} in practice. # Thread C calls {{allocateBytes}} with {{totalSize == size_C}} and exit with the return value {{size_B - 1}} and and {{position == size_B + size_C -1}} > Reduce thread contention in CommitLogSegment and HintsBuffer by rewriting CAS > loops to atomic adds > -------------------------------------------------------------------------------------------------- > > Key: CASSANDRA-16072 > URL: https://issues.apache.org/jira/browse/CASSANDRA-16072 > Project: Cassandra > Issue Type: Improvement > Components: Consistency/Hints, Local/Commit Log > Reporter: Michael Semb Wever > Assignee: Michael Semb Wever > Priority: Normal > Fix For: 3.11.x, 4.0-beta > > > Follow up to CASSANDRA-15922 > Both CommitLogSegment and HintsBuffer use AtomicIntegers for the current > offset when allocating. Like in CASSANDRA\-15922 the loops on > {{.compareAndSet(..)}} can be replaced with atomic adds using the {{. > getAndAdd(..)}} method. > In highly contended environments the CAS failures can be high, starving > writes in a running Cassandra node. On the same cluster CASSANDRA\-15922 was > found, after CASSANDRA\-15922's fix was deployed, there was still problems > around commit log flushing and hints. No flamegraph was collected that > demonstrated the thread contention as clearly as was found in > CASSANDRA\-15922, but the performance fix proposed here hopefully is obvious > enough. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org