Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
edoardocomar commented on PR #16151: URL: https://github.com/apache/kafka/pull/16151#issuecomment-2147303894 I think this also fixes https://issues.apache.org/jira/browse/KAFKA-16488 -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
edoardocomar commented on PR #16151: URL: https://github.com/apache/kafka/pull/16151#issuecomment-2147247109 merged in trunk, cherry-picked to 3.7.1 and 3.8.0. thanks @gharris1727 -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
edoardocomar merged PR #16151: URL: https://github.com/apache/kafka/pull/16151 -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
edoardocomar commented on PR #16151: URL: https://github.com/apache/kafka/pull/16151#issuecomment-2147128804 Jenkins PR Jdk8 connect failure retested manually ``` ./gradlew --no-daemon --no-build-cache cleantest connect:runtime:test --tests "ConnectorRestartApiIntegrationTest.testMultiWorkerRestartBothConnectorAndTasks" ``` `PASSED` -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
edoardocomar commented on PR #16151: URL: https://github.com/apache/kafka/pull/16151#issuecomment-2146353225 > If someone wants to raise the timeout for this one operation, I don't think that we should require them to increase the client-global request.timeout.ms. I agree to that. Hopefully the commit addresses your concerns. -- 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.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
gharris1727 commented on code in PR #16151:
URL: https://github.com/apache/kafka/pull/16151#discussion_r1624740901
##
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##
@@ -4569,7 +4569,7 @@ public ListTransactionsResult
listTransactions(ListTransactionsOptions options)
public FenceProducersResult fenceProducers(Collection
transactionalIds, FenceProducersOptions options) {
AdminApiFuture.SimpleAdminApiFuture future =
FenceProducersHandler.newFuture(transactionalIds);
-FenceProducersHandler handler = new FenceProducersHandler(logContext);
+FenceProducersHandler handler = new FenceProducersHandler(logContext,
requestTimeoutMs);
Review Comment:
If the AdminClient is no longer waiting for the fence request to complete,
does it make sense for the underlying append to continue?
If the AdminClient is willing to wait longer for the fence request to
complete, should the underlying append be aborted early?
If someone wants to raise the timeout for this one operation, I don't think
that we should require them to increase the client-global request.timeout.ms.
--
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.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
edoardocomar commented on code in PR #16151:
URL: https://github.com/apache/kafka/pull/16151#discussion_r1624103535
##
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##
@@ -4569,7 +4569,7 @@ public ListTransactionsResult
listTransactions(ListTransactionsOptions options)
public FenceProducersResult fenceProducers(Collection
transactionalIds, FenceProducersOptions options) {
AdminApiFuture.SimpleAdminApiFuture future =
FenceProducersHandler.newFuture(transactionalIds);
-FenceProducersHandler handler = new FenceProducersHandler(logContext);
+FenceProducersHandler handler = new FenceProducersHandler(logContext,
requestTimeoutMs);
Review Comment:
Hi @gharris1727 I'd appreciate if you can take a 2nd look or approve as IMHO
without this small fix, exactly once MM2/connect is unusable in production, so
I'd like to have this in 3.8.0 and 3.7.1
--
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.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
edoardocomar commented on code in PR #16151:
URL: https://github.com/apache/kafka/pull/16151#discussion_r1624103535
##
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##
@@ -4569,7 +4569,7 @@ public ListTransactionsResult
listTransactions(ListTransactionsOptions options)
public FenceProducersResult fenceProducers(Collection
transactionalIds, FenceProducersOptions options) {
AdminApiFuture.SimpleAdminApiFuture future =
FenceProducersHandler.newFuture(transactionalIds);
-FenceProducersHandler handler = new FenceProducersHandler(logContext);
+FenceProducersHandler handler = new FenceProducersHandler(logContext,
requestTimeoutMs);
Review Comment:
Hi @gharris1727 I'd appreciate if you can take a 2nd look or approve as IMHO
without this small fix, exactly once MM2/connect is unusable in production
--
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.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
edoardocomar commented on code in PR #16151:
URL: https://github.com/apache/kafka/pull/16151#discussion_r1623013225
##
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##
@@ -4569,7 +4569,7 @@ public ListTransactionsResult
listTransactions(ListTransactionsOptions options)
public FenceProducersResult fenceProducers(Collection
transactionalIds, FenceProducersOptions options) {
AdminApiFuture.SimpleAdminApiFuture future =
FenceProducersHandler.newFuture(transactionalIds);
-FenceProducersHandler handler = new FenceProducersHandler(logContext);
+FenceProducersHandler handler = new FenceProducersHandler(logContext,
requestTimeoutMs);
Review Comment:
https://issues.apache.org/jira/browse/KAFKA-16047?focusedCommentId=17851019
--
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.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
edoardocomar commented on code in PR #16151:
URL: https://github.com/apache/kafka/pull/16151#discussion_r1622769486
##
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##
@@ -4569,7 +4569,7 @@ public ListTransactionsResult
listTransactions(ListTransactionsOptions options)
public FenceProducersResult fenceProducers(Collection
transactionalIds, FenceProducersOptions options) {
AdminApiFuture.SimpleAdminApiFuture future =
FenceProducersHandler.newFuture(transactionalIds);
-FenceProducersHandler handler = new FenceProducersHandler(logContext);
+FenceProducersHandler handler = new FenceProducersHandler(logContext,
requestTimeoutMs);
Review Comment:
as an override to the AdminClient REQUEST_TIMEOUT_MS_CONFIG ?
The options doc says it's on override on API timeout
but yes we could use that too
--
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.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
gharris1727 commented on code in PR #16151:
URL: https://github.com/apache/kafka/pull/16151#discussion_r1622743028
##
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##
@@ -4569,7 +4569,7 @@ public ListTransactionsResult
listTransactions(ListTransactionsOptions options)
public FenceProducersResult fenceProducers(Collection
transactionalIds, FenceProducersOptions options) {
AdminApiFuture.SimpleAdminApiFuture future =
FenceProducersHandler.newFuture(transactionalIds);
-FenceProducersHandler handler = new FenceProducersHandler(logContext);
+FenceProducersHandler handler = new FenceProducersHandler(logContext,
requestTimeoutMs);
Review Comment:
There's a requestTimeoutMs in options, can you use that if specified?
--
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.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] KAFKA-16047: Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers [kafka]
edoardocomar opened a new pull request, #16151: URL: https://github.com/apache/kafka/pull/16151 Use REQUEST_TIMEOUT_MS_CONFIG in AdminClient.fenceProducers as transaction timeout. No transaction will be started with this timeout, but ReplicaManager.appendRecords uses this value as its timeout. Use REQUEST_TIMEOUT_MS_CONFIG like a regular producer append to allow for replication to take place. Co-Authored-By: Adrian Preston ### 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
