You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "vcrfxia (via GitHub)" <gi...@apache.org> on 2023/04/05 23:01:35 UTC

[GitHub] [kafka] vcrfxia opened a new pull request, #13509: KAFKA-14834: [3/N] Timestamped lookups for stream-table joins

vcrfxia opened a new pull request, #13509:
URL: https://github.com/apache/kafka/pull/13509

   (This PR is stacked on https://github.com/apache/kafka/pull/13496 and https://github.com/apache/kafka/pull/13497. The first two commits do not need to be reviewed separately.)
   
   This PR updates the stream-table join processors, including both KStream-KTable and KStream-GlobalKTable joins, to perform timestamped lookups when the (global) table is versioned, as specified in [KIP-914](https://cwiki.apache.org/confluence/display/KAFKA/KIP-914%3A+Join+Processor+Semantics+for+Versioned+Stores).
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] mjsax merged pull request #13509: KAFKA-14834: [3/N] Timestamped lookups for stream-table joins

Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax merged PR #13509:
URL: https://github.com/apache/kafka/pull/13509


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] vcrfxia commented on a diff in pull request #13509: KAFKA-14834: [3/N] Timestamped lookups for stream-table joins

Posted by "vcrfxia (via GitHub)" <gi...@apache.org>.
vcrfxia commented on code in PR #13509:
URL: https://github.com/apache/kafka/pull/13509#discussion_r1159109536


##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamGlobalKTableJoinTest.java:
##########
@@ -246,4 +262,30 @@ public void shouldJoinOnNullKeyWithNonNullKeyMapperValues() {
         processor.checkAndClearProcessResult(new KeyValueTimestamp<>(null, "X0,FKey0+Y0", 0),
                 new KeyValueTimestamp<>(1, "X1,FKey1+Y1", 1));
     }
+
+    @Test
+    public void shouldPerformTimestampedGet() {

Review Comment:
   It doesn't look like there's existing integration test coverage for KStream-GlobalKTable joins, so I've added these tests here instead. 
   
   For KStream-KTable joins, there's also `KStreamKTableJoinTest` and `KStreamKTableLeftJoinTest` which I did not update, since I opted to update `StreamTableJoinIntegrationTest` instead. AFAICT both sets of tests are run with the TopologyTestDriver so updating `KStreamKTableJoinTest` and `KStreamKTableLeftJoinTest` would be redundant to having updated `StreamTableJoinIntegrationTest`, but if that's not true then I can update those unit tests as well.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] vcrfxia commented on pull request #13509: KAFKA-14834: [3/N] Timestamped lookups for stream-table joins

Posted by "vcrfxia (via GitHub)" <gi...@apache.org>.
vcrfxia commented on PR #13509:
URL: https://github.com/apache/kafka/pull/13509#issuecomment-1515473590

   Closing the loop here: versioned stores are disallowed for GlobalKTables in https://github.com/apache/kafka/pull/13565, so we should be all set.


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] mjsax commented on pull request #13509: KAFKA-14834: [3/N] Timestamped lookups for stream-table joins

Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #13509:
URL: https://github.com/apache/kafka/pull/13509#issuecomment-1506112408

   Merged this PR, but as discussed in person, it seems questionable to allow versioned-stores for global-ktables -- the code change to the stream-table join processor that is shared for stream-table and stream-globalTable join is fine, but when we disallow versioned stores for global-tables, we would need to revert the corresponding test cases in a follow up PR.


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org