[ 
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

Reply via email to