C0urante merged PR #13801:
URL: https://github.com/apache/kafka/pull/13801
--
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-unsubscr...@kafka.apache
vamossagar12 commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-2092565347
Thanks @C0urante , good catch, yeah those are missing. I have modified some
of the tests to consider all the 3 types of offset records. I added another
test for the case when write t
C0urante commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1583356709
##
connect/runtime/src/test/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStoreTest.java:
##
@@ -0,0 +1,410 @@
+/*
+ * Licensed to the Apache Software Fo
vamossagar12 commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-2076983151
Thanks Chris! I ran through the scenarios in the test and I can see that it
handles the cases correctly. Regarding, `cancel` I don't see the future
returned from `set` being cancelle
C0urante commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-2056973237
Thanks Sagar, great catch! I suspected this would be a gnarly one to tackle
but it's turning out to be even harder than I thought.
I think there's still an issue with the current s
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1560724752
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +300,77 @@ public Future set(Map
values, Callbac
vamossagar12 commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-2049281246
@C0urante , I think I figured out the reason for the failure with
`testFlushFailureWhenWritesToPrimaryStoreFailsAndSecondarySucceedsForTombstoneRecords`.
The problem was that from we
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1560701187
##
checkstyle/suppressions.xml:
##
@@ -166,7 +166,7 @@
files="(KafkaConfigBackingStore|Values|ConnectMetricsRegistry).java"/>
+
vamossagar12 commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-2047548232
Chris, I started changing the tests in alignment with the comments (i.e
using AtomicBoolean, AtomicReference and removing try-catch block). I noticed
an interesting issue with
`test
C0urante commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1555264597
##
connect/runtime/src/test/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStoreTest.java:
##
@@ -0,0 +1,291 @@
+/*
+ * Licensed to the Apache Software Fo
vamossagar12 commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-2039360963
Hey Chris, sorry for the long delay on this. I finally got a chance to
verify the code that you provided and it makes sense. I agree that so far I was
only thinking about either havi
vamossagar12 commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-1921517994
Thanks @C0urante for this! Let me review this and get back to you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub an
C0urante commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-1919475280
Thanks for the in-depth analysis! I think part of the problem here stems
from trying to make our internal `Future`-based API cooperate with Java's
`CompleteableFuture` API. I've sketched
vamossagar12 commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-1908434176
@C0urante , I did some analysis today pertaining to the version that was
present with `CompletableFuture`
[here](https://github.com/apache/kafka/blob/062591757a04647eb4837348f59b0e57
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463150319
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callbac
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463150319
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callbac
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463158103
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callbac
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463165311
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callbac
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463165311
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callbac
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463165311
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callbac
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463158347
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -299,12 +349,11 @@ public Future set(Map
values, Callbac
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463158103
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callbac
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463150319
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callbac
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463144968
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callbac
vamossagar12 commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1463143759
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callbac
vamossagar12 commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-1848322802
> Thanks @vamossagar12, I've given the functional changes another pass.
Unless I'm mistaken about the semantics for `CompletableFuture::thenAccept`,
there's a pretty serious bug in t
C0urante commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1417751374
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callback ca
C0urante commented on code in PR #13801:
URL: https://github.com/apache/kafka/pull/13801#discussion_r1417677509
##
connect/runtime/src/main/java/org/apache/kafka/connect/storage/ConnectorOffsetBackingStore.java:
##
@@ -279,15 +290,54 @@ public Future set(Map
values, Callback ca
C0urante commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-1842053883
Removed the stale label.
@vamossagar12 thanks for your patience on this one, I plan on making another
pass this week!
--
This is an automated message from the Apache Git Service
github-actions[bot] commented on PR #13801:
URL: https://github.com/apache/kafka/pull/13801#issuecomment-1842029141
This PR is being marked as stale since it has not had any activity in 90
days. If you would like to keep this PR alive, please ask a committer for
review. If the PR has merge
30 matches
Mail list logo
