KAFKA-5054 : synchronized all the methods

[lijubjohn]

This avoids inconsistent results while querying or storing the underlying storage

[viktorsomogyi]

@lijubjohn if you don't mind I reviewed it: it looks fine.
To be a bit more elaborate: from a bit higher level though I think it that while this is a good tactical fix we might want to measure performance impact (if these are frequently used methods) and maybe come up with a better design where synchronization is implicit such as immutable LoggedStateChange classes which would make sure that logging and storing the change are done atomicly.

[mjsax]

Thanks for the PR @lijubjohn -- I was reading the ticket, but I am not sure if I fully understand the issue to be honest. The ticket mentions IQ and that putIfAbsent and delete do two operation -- however, IQ only allows read access anyway, and thus I don't understand.

@dguy You created the original ticket. Can you help out here?

Also, for IQ we actually synchronized store access state store level (ie, RocksDBStore and InMemoryKeyValueStore already.

[guozhangwang]

@mjsax @lijubjohn I think here is the issue @dguy reported:

Consider the putIfAbsent call:

        final byte[] previous = get(key);  // 1
        if (previous == null) {                    // 2
            put(key, value);
        }
        return previous;

It is possible that a concurrent get call is triggered between 1 and 2. The IQ query will return none while at the end it the key will have some values instead; similar situations for delete.

But thinking about this, this could be just treated as "the IQ get call was triggered before the putIfAbsent", since we do not guarantee strict time-ordering for such concurrent operations anyways. So I think we do not necessarily need to fix the JIRA.

[guozhangwang]

@mjsax if you agree we can close the JIRA as not a bug.

[mjsax]

I agree with this. The result must be the same as an serial execution (not specified with one), and this seems to be guaranteed as you pointed out.

[guozhangwang]

Hi @lijubjohn as discussed about I'm going to close the ticket as not a bug. Please feel free to pick up other tickets from https://issues.apache.org/jira/issues/?jql=project%20%3D%20KAFKA%20AND%20resolution%20%3D%20Unresolved%20AND%20component%20%3D%20streams%20ORDER%20BY%20priority%20DESC

thanks for your contributions again!