You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "satishd (via GitHub)" <gi...@apache.org> on 2023/02/12 13:57:23 UTC
[GitHub] [kafka] satishd opened a new pull request, #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
satishd opened a new pull request, #13234:
URL: https://github.com/apache/kafka/pull/13234
KAFKA-14706 Move/rewrite ShutdownableThread to `server-common` module.
This change is in progress and not yet ready for review.
### 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] showuon merged pull request #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon merged PR #13234:
URL: https://github.com/apache/kafka/pull/13234
--
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] showuon commented on pull request #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on PR #13234:
URL: https://github.com/apache/kafka/pull/13234#issuecomment-1433985820
I'll take a look today.
--
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] satishd commented on pull request #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
Posted by "satishd (via GitHub)" <gi...@apache.org>.
satishd commented on PR #13234:
URL: https://github.com/apache/kafka/pull/13234#issuecomment-1433080348
Thanks @ijuma and @showuon for the review. Resolved the conflicts and pushed the changes.
--
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] satishd commented on a diff in pull request #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
Posted by "satishd (via GitHub)" <gi...@apache.org>.
satishd commented on code in PR #13234:
URL: https://github.com/apache/kafka/pull/13234#discussion_r1104124125
##########
examples/src/main/java/kafka/examples/Consumer.java:
##########
@@ -83,13 +83,4 @@ public void doWork() {
}
}
- @Override
- public String name() {
Review Comment:
+1 "examples" module should not be dependant on internal server side components, filed https://issues.apache.org/jira/browse/KAFKA-14708
--
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] showuon commented on a diff in pull request #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on code in PR #13234:
URL: https://github.com/apache/kafka/pull/13234#discussion_r1105364379
##########
examples/src/main/java/kafka/examples/Consumer.java:
##########
@@ -83,13 +83,4 @@ public void doWork() {
}
}
- @Override
- public String name() {
Review Comment:
Good point!
--
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] satishd commented on pull request #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
Posted by "satishd (via GitHub)" <gi...@apache.org>.
satishd commented on PR #13234:
URL: https://github.com/apache/kafka/pull/13234#issuecomment-1434009975
One failing test is not related to this change, ready to be merged.
--
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] ijuma commented on a diff in pull request #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #13234:
URL: https://github.com/apache/kafka/pull/13234#discussion_r1103848755
##########
server-common/src/main/java/org/apache/kafka/server/util/ShutdownableThread.java:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kafka.server.util;
+
+import org.apache.kafka.common.internals.FatalExitError;
+import org.apache.kafka.common.utils.Exit;
+import org.apache.kafka.common.utils.LogContext;
+import org.slf4j.Logger;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+public abstract class ShutdownableThread extends Thread {
+
+ public final String logPrefix;
+
+ private final Logger log;
+
+ private final boolean isInterruptible;
+
+
+ private CountDownLatch shutdownInitiated = new CountDownLatch(1);
+ private CountDownLatch shutdownComplete = new CountDownLatch(1);
Review Comment:
I know this is still a draft, but in case you haven't noticed, `final` is missing from here.
--
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] ijuma commented on a diff in pull request #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #13234:
URL: https://github.com/apache/kafka/pull/13234#discussion_r1103848236
##########
examples/src/main/java/kafka/examples/Consumer.java:
##########
@@ -83,13 +83,4 @@ public void doWork() {
}
}
- @Override
- public String name() {
Review Comment:
Let's file a ticket in JIRA for removing the dependency on `ShutdownableThread` - our examples should not rely on internal apis.
--
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] satishd commented on pull request #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
Posted by "satishd (via GitHub)" <gi...@apache.org>.
satishd commented on PR #13234:
URL: https://github.com/apache/kafka/pull/13234#issuecomment-1428095317
Test failure does not seem to be related to the changes in the 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
[GitHub] [kafka] ijuma commented on pull request #13234: KAFKA-14706 Move/rewrite ShutdownableThread to server-common module.
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #13234:
URL: https://github.com/apache/kafka/pull/13234#issuecomment-1433083812
I won't have a chance to review for a few more days - if @showuon can do it sooner, let's not block on me.
--
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