Sorry if this is long, it includes my thoughts and comments (helpful if you guys could correct things that are incorrect, so I can learn).  There is a brief TLDR at the end.

Checking the state transfer lock should work it sounds like.  It could do a check to see if the data container contained something and if not lock and then query the data container again and only write to if it didn't have a value.

1. I would agree it would eventually be the same result, however at a big cost IMHO.  See last paragraph.  And unfortunately we can't just acquire the key before the remote get without breaking encapsulation, as L1 logic would have to go back into the dist interceptor again so it wouldn't lock for reads that didn't have to go remote when L1 is enabled.

2. I agree on the put commands originating from the local node.  That is covered in test: testNoEntryInL1GetWithConcurrentPut.  To be honest after reading Galder's comments I looked over the code again and found this as well, and I think the reason it is working is because of a bug that any additional owners that aren't primary always send invalidation since the origin is the primary node and not the originating node anymore.  Your point is a simple fix though and just requires the same logic as the L1 invalidation to ensure the get is completed.  To be honest I have a change with that locally because I was working on TX L1 changes that required this already.

3a. The zero timeout is hard coded [1]
Without fixing the bug in #2 this would cause deadlock for the lock duration as well for sync caches - but no objection there.  Thinking more about it we can't increase the timeout in a sync cache and add locking everywhere.  Let me explain an example with sync caches

Node B owns k with a value of v1
Node A has k in it's L1

Node A initiates put(k, v2) - acquires local lock on A - hasn't reached B yet
Node B inititiates put(k, v3) - acquires lock for k sends invalidation to Node A which now blocks until timeout
30 seconds later or whatever timeout
Node A finally acquires lock for k on Node B and updates

This also makes me think of an edge case that if you have a optimistic sync cache and a L1 invalidation comes in right after a prepare has locked the keys locally and if that commit fails for whever reason (write skew, etc) that it will not have removed the stale L1 value.  I would need to test this though.


3b. That seems a bit heavy handed to me and I thought we wanted lock free gets? In this case we have to add locking to all operations(read and write) to that key (or stripe).  Currently local locks are not even acquired for writes if you take a look at [2]  (only on primary node for the update duration).  This could cause heavy contention especially when lock striping is enabled(this approach has no additional overhead for lock striping).

Before my changes there are only 2 operations that acquire the local lock - get when updating L1 and L1 invalidation; Write operations don't acquire the local lock and just remove the value blindly (bug?).  Adding full locking will be a much larger overhead than one would initially think as you now have to effectively serialize network traffic for every call, where as non locking those network calls were done in parallel and the owner sorts out the order it retrieved them in.  Also in the case of lock striping this will make it much much worse. 

Like you mentioned if we have to go to this much locking it seems L1 should probably be disabled when there is expected concurrent access including writes to a key or when lock striping is enabled when using full locking if it even works.

[1] - https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/interceptors/locking/AbstractLockingInterceptor.java#L107

[2] -https://github.com/infinispan/infinispan/blob/master/core/src/main/java/org/infinispan/interceptors/locking/AbstractLockingInterceptor.java#L137

Also I don't know if you guys wanted to possibly move this to dev mailing list?

TLDR;

We can't use a try lock approach for L1 invalidation when gets are locked without possibly dropping a L1 update
We can't use a timed lock approach for L1 invalidation when writes lock the key as we could get into a timed deadlock situation when another node concurrently writes to a value/stripe