[ 
https://issues.apache.org/jira/browse/CASSANDRA-14871?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705521#comment-16705521
 ] 

Blake Eggleston commented on CASSANDRA-14871:
---------------------------------------------

I was taking a look at the 3.0 changes and had a few comments, I'll take a look 
at the other versions on Monday. Sorry if I'm stepping on your toes [~jkni], 
I'm also happy to take over the review if you're busy.

DateTieredCompactionStrategy
 * access to {{sstables}} is unguarded in {{getNextBackgroundSSTables}}

TokenMetadata
 * {{@VisibleForTesting}} annotation on topology field could be removed
 * Could you either use a read lock in {{TokenMetadata#getTopology}}, or rename 
it getTopologyUnsafe? I realize it’s not used in a way might have visibility 
issues at the moment, but it is silently deviating from the concurrency 
strategy used everywhere else for that field. It should either conform to the 
strategy, or have a name scary enough to make people read the method docs 
before using it. (I’d prefer just using a read lock for simplicity/consistency)
 * Since we’re now using copy on write for {{TokenMetadata.Topology}} changes 
and treating {{Topology}} as immutable, it would be clearer if the mutating 
methods were split out into a builder class and the Topology only exposed 
getter type methods. This might be overkill for 3.x, but it makes sense for 
trunk.

> Severe concurrency issues in STCS,DTCS,TWCS,TMD.Topology,TypeParser
> -------------------------------------------------------------------
>
>                 Key: CASSANDRA-14871
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14871
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Core
>            Reporter: Robert Stupp
>            Assignee: Robert Stupp
>            Priority: Critical
>             Fix For: 4.0, 3.0.x, 3.11.x
>
>
> There are a couple of places in the code base that do not respect that 
> j.u.HashMap + related classes are not thread safe and some parts rely on 
> internals of the implementation of HM, which can change.
> We have observed failures like {{NullPointerException}} and  
> {{ConcurrentModificationException}} as well as wrong behavior.
> Affected areas in the code base:
>  * {{SizeTieredCompactionStrategy}}
>  * {{DateTieredCompactionStrategy}}
>  * {{TimeWindowCompactionStrategy}}
>  * {{TokenMetadata.Topology}}
>  * {{TypeParser}}
>  * streaming / concurrent access to {{LifecycleTransaction}} (handled in 
> CASSANDRA-14554)
> While the patches for the compaction strategies + {{TypeParser}} are pretty 
> straight forward, the patch for {{TokenMetadata.Topology}} requires it to be 
> made immutable.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to