315157973 commented on pull request #9850:
URL: https://github.com/apache/pulsar/pull/9850#issuecomment-796505929
> It's great that the thread safely of the data structures is now handled so
we won't run into infinite loops which would happen with plain HashMaps. With
the ConcurrentHashMaps, it will also be possible to handle data consistency
issues.
>
> There are some remaining data consistency ("lost update") issues when
mutating `AuthPolicies` maps, for example here:
>
>
https://github.com/apache/pulsar/blob/e24645852edeee6aa0850ce814384bedd6518e7a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java#L691-L694
>
>
https://github.com/apache/pulsar/blob/1fab5aa69446b4c2eaebb615d35fe9f5a509ba4c/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java#L296-L299
>
>
https://github.com/apache/pulsar/blob/782372d8f51984f18de3b102cfb3c00c6dedb2bb/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java#L344-L356
>
>
https://github.com/apache/pulsar/blob/76883d455b44b7b0b73684d0212002397dbb9b2c/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L324-L331
>
>
https://github.com/apache/pulsar/blob/76883d455b44b7b0b73684d0212002397dbb9b2c/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L391-L395
>
> There might be some other locations besides the ones above.
>
> Would it help to use `Map.compute` instead of `.get(..).removeAll(...)`
and `.put(..,..)` for mutations? Isn't `Map.compute` atomic for
`ConcurrentHashMap`? WDYT?
>
> @315157973 are you thinking of addressing the mutations of the
`AuthPolicies` maps in this PR or separately?
The solution I thought of:
I will start a new PR to add zk version to the policies cache.
When the policies cache in some thread is dirty(with old version), writing
to zk will report an error. At this time, the latest cache will be read, and
then this Function will be called again to ensure that the dirty cache will not
overwrite the new data.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
