Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-11 Thread via GitHub
dajac merged PR #15142: URL: https://github.com/apache/kafka/pull/15142 -- 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.or

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1448416633 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,57 @@ class CoordinatorPartitionWriter[T]( )) } + /** + *

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
artemlivshits commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r144830 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1448027564 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** +

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1447988893 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** +

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1447918075 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,57 @@ class CoordinatorPartitionWriter[T]( )) } + /** + *

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1447916841 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** + *

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1447883700 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,57 @@ class CoordinatorPartitionWriter[T]( )) } + /** +

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1447880085 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** +

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1447879037 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** +

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1447873603 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorService.java: ## @@ -1106,6 +1106,8 @@ private static boolean isGroupIdNotEmpty(St

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
dajac commented on PR #15142: URL: https://github.com/apache/kafka/pull/15142#issuecomment-1884851533 @jolshan Thanks for your comments. I have addressed all of them expect the one about the thread re-scheduling. I propose to tackle this separately in order to keep this patch simple. I have

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1447125608 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupCoordinatorService.java: ## @@ -1106,6 +1106,8 @@ private static boolean isGroupIdNotEmpty(Stri

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1447124222 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** + *

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-10 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1447050169 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/PartitionWriter.java: ## @@ -116,4 +120,21 @@ long appendEndTransactionMarker( int

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-09 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1446688518 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** +

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-09 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1446663978 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** + *

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-09 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1446657520 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** +

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-09 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1446654869 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** + *

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-09 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1446538966 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/PartitionWriter.java: ## @@ -116,4 +120,21 @@ long appendEndTransactionMarker( in

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-09 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1446536290 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** +

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-09 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1446091865 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** + *

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-09 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1446091535 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/PartitionWriter.java: ## @@ -116,4 +120,21 @@ long appendEndTransactionMarker( int

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-09 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1446088963 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** + *

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-09 Thread via GitHub
dajac commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1446089431 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java: ## @@ -1466,17 +1477,25 @@ public CompletableFuture scheduleTrans

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-08 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1445444996 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** +

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-08 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1445431424 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/PartitionWriter.java: ## @@ -116,4 +120,21 @@ long appendEndTransactionMarker( in

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-08 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1445426296 ## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java: ## @@ -1466,17 +1477,25 @@ public CompletableFuture scheduleTra

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-08 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1445415307 ## core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala: ## @@ -201,18 +203,55 @@ class CoordinatorPartitionWriter[T]( )) } + /** +

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-08 Thread via GitHub
jolshan commented on code in PR #15142: URL: https://github.com/apache/kafka/pull/15142#discussion_r1445412684 ## core/src/main/scala/kafka/server/ReplicaManager.scala: ## @@ -989,6 +989,7 @@ class ReplicaManager(val config: KafkaConfig, * @param requestLocal

Re: [PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-08 Thread via GitHub
jolshan commented on PR #15142: URL: https://github.com/apache/kafka/pull/15142#issuecomment-1881648674 Yup -- sorry was hoping to do all the clean ups before you got to this -- but shouldn't be too hard to update. -- This is an automated message from the Apache Git Service. To respond to

[PR] KAFKA-14505; [4/N] Wire transaction verification [kafka]

2024-01-08 Thread via GitHub
dajac opened a new pull request, #15142: URL: https://github.com/apache/kafka/pull/15142 This patch wires the transaction verification in the new group coordinator. It basically calls the verification path before scheduling the write operation. If the verification fails, the error is return