twalthr commented on PR #29421:
URL: https://github.com/apache/flink/pull/29421#issuecomment-6125844300

   Additional feedback:
   
   
   1. With no equi-key, state_ttl evicts the whole build table at once.
   - In stream mode all rows share one empty key, so the TTL timer 
(LateralSnapshotJoinOperator#onProcessingTime → clearAllPerKeyState()) applies 
to the entire dimension table.
   - While any probe or build traffic arrives, the TTL timer keeps being reset, 
so nothing expires.
   - After a quiet period longer than the TTL, every build row is dropped 
together. Later probes then see only the build rows changed after eviction: 
INNER drops rows, LEFT null-pads them. Without the TTL that would not happen.
   - The docs (joins.md:414) still say "Join keys that are not accessed … 
become eligible for eviction", which doesn't describe the keyless case.
   - Fix: document this behaviour. Also consider logging or warning when a 
keyless join is combined with a TTL. There is no test for TTL eviction in 
empty-key mode.
   
   2. The docs and the commit message are wrong about batch.
   - Both say a keyless join "runs single-threaded".
   - In batch, the new code emits a broadcast nested-loop join that keeps the 
probe side parallel, as the BatchPhysicalLateralSnapshotJoinRule Javadoc itself 
says. Only streaming is single-threaded.
   - The commit message says "a keyless join runs single-threaded - SINGLETON 
distribution in streaming … and a broadcast nested-loop join in batch", which 
contradicts itself.
   - Fix: reword both to make clear the single-threaded case is streaming only.


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to