You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2023/01/09 05:08:49 UTC
[GitHub] [kafka] ijuma opened a new pull request, #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
ijuma opened a new pull request, #13092:
URL: https://github.com/apache/kafka/pull/13092
For broader context, see:
* KAFKA-14470: Move log layer to storage module
### 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] ijuma commented on pull request #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on PR #13092:
URL: https://github.com/apache/kafka/pull/13092#issuecomment-1378356657
JDK 17 build passed and the other two had unrelated failures:
> Build / JDK 8 and Scala 2.12 / org.apache.kafka.streams.integration.SmokeTestDriverIntegrationTest.[1] true
> Build / JDK 8 and Scala 2.12 / org.apache.kafka.streams.integration.SmokeTestDriverIntegrationTest.[2] false
> Build / JDK 11 and Scala 2.13 / org.apache.kafka.clients.consumer.StickyAssignorTest.testLargeAssignmentAndGroupWithNonEqualSubscription()
> Build / JDK 11 and Scala 2.13 / org.apache.kafka.clients.consumer.internals.DefaultBackgroundThreadTest.testStartupAndTearDown()
> Build / JDK 11 and Scala 2.13 / kafka.api.TransactionsTest.testBumpTransactionalEpoch(String).quorum=kraft
> Build / JDK 11 and Scala 2.13 / org.apache.kafka.streams.integration.StoreQueryIntegrationTest.shouldQueryStoresAfterAddingAndRemovingStreamThread()
> Build / JDK 11 and Scala 2.13 / org.apache.kafka.streams.processor.internals.DefaultStateUpdaterTest.shouldRemovePausedAndUpdatingTasksOnShutdown()
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1064289083
##########
core/src/main/scala/kafka/log/LogManager.scala:
##########
@@ -1363,7 +1356,7 @@ object LogManager {
def apply(config: KafkaConfig,
initialOfflineDirs: Seq[String],
configRepository: ConfigRepository,
- kafkaScheduler: KafkaScheduler,
+ kafkaScheduler: Scheduler,
Review Comment:
We should use the interface instead of the concrete class.
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1064288489
##########
core/src/main/scala/kafka/coordinator/group/GroupMetadataManager.scala:
##########
@@ -974,8 +975,7 @@ class GroupMetadataManager(brokerId: Int,
def shutdown(): Unit = {
shuttingDown.set(true)
- if (scheduler.isStarted)
- scheduler.shutdown()
+ scheduler.shutdown()
Review Comment:
It's redundant to check if the scheduler has started before calling `shutdown`.
--
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] junrao commented on a diff in pull request #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
junrao commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1066374013
##########
server-common/src/main/java/org/apache/kafka/server/util/KafkaScheduler.java:
##########
@@ -0,0 +1,188 @@
+/*
+ * 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.utils.KafkaThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.concurrent.Delayed;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * A scheduler based on java.util.concurrent.ScheduledThreadPoolExecutor
+ *
+ * It has a pool of kafka-scheduler- threads that do the actual work.
+ */
+public class KafkaScheduler implements Scheduler {
+
+ private static class NoOpScheduledFutureTask implements ScheduledFuture<Void> {
+
+ @Override
+ public boolean cancel(boolean mayInterruptIfRunning) {
+ return true;
+ }
+
+ @Override
+ public boolean isCancelled() {
+ return true;
+ }
+
+ @Override
+ public boolean isDone() {
+ return true;
+ }
+ @Override
Review Comment:
add a newline above.
##########
core/src/main/scala/kafka/log/LogManager.scala:
##########
@@ -1049,16 +1045,13 @@ class LogManager(logDirs: Seq[File],
error(s"Exception in kafka-delete-logs thread.", e)
} finally {
try {
- scheduler.schedule("kafka-delete-logs",
+ scheduler.scheduleOnce("kafka-delete-logs",
deleteLogs _,
- delay = nextDelayMs,
- unit = TimeUnit.MILLISECONDS)
+ nextDelayMs)
} catch {
case e: Throwable =>
- if (scheduler.isStarted) {
Review Comment:
Thanks for catching this. #11351 changed the scheduler to return NoOpScheduledFutureTask if the scheduler is not started. But we forgot to change the code here accordingly.
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1064288206
##########
core/src/main/scala/kafka/controller/KafkaController.scala:
##########
@@ -458,8 +459,9 @@ class KafkaController(val config: KafkaConfig,
}
private def scheduleAutoLeaderRebalanceTask(delay: Long, unit: TimeUnit): Unit = {
- kafkaScheduler.schedule("auto-leader-rebalance-task", () => eventManager.put(AutoPreferredReplicaLeaderElection),
- delay = delay, unit = unit)
+ kafkaScheduler.scheduleOnce("auto-leader-rebalance-task",
+ () => eventManager.put(AutoPreferredReplicaLeaderElection),
+ unit.toMillis(delay))
Review Comment:
This is the only case where we were calling `schedule` with a unit that is not `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-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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1064289351
##########
core/src/main/scala/kafka/server/KafkaBroker.scala:
##########
@@ -74,7 +74,7 @@ trait KafkaBroker extends KafkaMetricsGroup {
def config: KafkaConfig
def dataPlaneRequestHandlerPool: KafkaRequestHandlerPool
def dataPlaneRequestProcessor: KafkaApis
- def kafkaScheduler: KafkaScheduler
+ def kafkaScheduler: Scheduler
Review Comment:
We should use the interface instead of the concrete class.
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1064288973
##########
core/src/main/scala/kafka/log/LogManager.scala:
##########
@@ -1049,16 +1045,13 @@ class LogManager(logDirs: Seq[File],
error(s"Exception in kafka-delete-logs thread.", e)
} finally {
try {
- scheduler.schedule("kafka-delete-logs",
+ scheduler.scheduleOnce("kafka-delete-logs",
deleteLogs _,
- delay = nextDelayMs,
- unit = TimeUnit.MILLISECONDS)
+ nextDelayMs)
} catch {
case e: Throwable =>
- if (scheduler.isStarted) {
Review Comment:
Need to double check if this change makes sense.
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1065902609
##########
server-common/src/test/java/org/apache/kafka/server/util/MockScheduler.java:
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.utils.Time;
+
+import java.util.Comparator;
+import java.util.Optional;
+import java.util.PriorityQueue;
+import java.util.concurrent.Delayed;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.function.Predicate;
+
+/**
+ * A mock scheduler that executes tasks synchronously using a mock time instance. Tasks are executed synchronously when
+ * the time is advanced. This class is meant to be used in conjunction with MockTime.
+ *
+ * Example usage
+ * <code>
+ * val time = new MockTime
+ * time.scheduler.schedule("a task", println("hello world: " + time.milliseconds), delay = 1000)
+ * time.sleep(1001) // this should cause our scheduled task to fire
+ * </code>
+ *
+ * Incrementing the time to the exact next execution time of a task will result in that task executing (it as if execution itself takes no time).
+ */
+public class MockScheduler implements Scheduler {
+
+ private static class MockTask implements ScheduledFuture<Void> {
+ final String name;
+ final Runnable task;
+ final long period;
+ final Time time;
+
+ private final AtomicLong nextExecution;
+
+ private MockTask(String name, Runnable task, long nextExecution, long period, Time time) {
+ this.name = name;
+ this.task = task;
+ this.nextExecution = new AtomicLong(nextExecution);
+ this.period = period;
+ this.time = time;
+ }
+
+ /**
+ * If this task is periodic, reschedule it and return true. Otherwise, do nothing and return false.
+ */
+ public boolean rescheduleIfPeriodic() {
Review Comment:
Extracted this logic into its own method.
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1065891467
##########
build.gradle:
##########
@@ -1921,6 +1921,7 @@ project(':streams') {
testImplementation project(':clients').sourceSets.test.output
testImplementation project(':core')
testImplementation project(':core').sourceSets.test.output
+ testImplementation project(':server-common').sourceSets.test.output
Review Comment:
The streams integration harness has a transitive runtime dependency to `Scheduler/KafkaScheduler` and dependencies to test jars are not transitive.
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on PR #13092:
URL: https://github.com/apache/kafka/pull/13092#issuecomment-1377438934
All builds passed. @junrao this is ready for review when you have cycles.
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1064289083
##########
core/src/main/scala/kafka/log/LogManager.scala:
##########
@@ -1363,7 +1356,7 @@ object LogManager {
def apply(config: KafkaConfig,
initialOfflineDirs: Seq[String],
configRepository: ConfigRepository,
- kafkaScheduler: KafkaScheduler,
+ kafkaScheduler: Scheduler,
Review Comment:
Using the interface instead of the concrete class.
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1064291557
##########
server-common/src/main/java/org/apache/kafka/server/util/Scheduler.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * 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 java.util.concurrent.ScheduledFuture;
+
+/**
+ * A scheduler for running jobs
+ *
+ * This interface controls a job scheduler that allows scheduling either repeating background jobs
+ * that execute periodically or delayed one-time actions that are scheduled in the future.
+ */
+public interface Scheduler {
+
+ /**
+ * Initialize this scheduler so it is ready to accept scheduling of tasks
+ */
+ void startup();
+
+ /**
+ * Shutdown this scheduler. When this method is complete no more executions of background tasks will occur.
+ * This includes tasks scheduled with a delayed execution.
+ */
+ void shutdown() throws InterruptedException;
+
+ default ScheduledFuture<?> scheduleOnce(String name, Runnable task) {
+ return scheduleOnce(name, task, 0L);
+ }
+
+ default ScheduledFuture<?> scheduleOnce(String name, Runnable task, long delayMs) {
Review Comment:
In Java we have to use multiple methods (potentially with overloads) instead of default arguments. For clarity, we use `scheduleOnce` for the case where the task is executed only once (`periodMs < 0`).
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1064288335
##########
core/src/main/scala/kafka/controller/KafkaController.scala:
##########
@@ -479,8 +481,7 @@ class KafkaController(val config: KafkaConfig,
kafkaScheduler.shutdown()
// stop token expiry check scheduler
- if (tokenCleanScheduler.isStarted)
- tokenCleanScheduler.shutdown()
+ tokenCleanScheduler.shutdown()
Review Comment:
It's safe to call `shutdown` even if `isStarted` was not called.
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1065902085
##########
server-common/src/test/java/org/apache/kafka/server/util/MockScheduler.java:
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.utils.Time;
+
+import java.util.Comparator;
+import java.util.Optional;
+import java.util.PriorityQueue;
+import java.util.concurrent.Delayed;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.function.Predicate;
+
+/**
+ * A mock scheduler that executes tasks synchronously using a mock time instance. Tasks are executed synchronously when
+ * the time is advanced. This class is meant to be used in conjunction with MockTime.
+ *
+ * Example usage
+ * <code>
+ * val time = new MockTime
+ * time.scheduler.schedule("a task", println("hello world: " + time.milliseconds), delay = 1000)
+ * time.sleep(1001) // this should cause our scheduled task to fire
+ * </code>
+ *
+ * Incrementing the time to the exact next execution time of a task will result in that task executing (it as if execution itself takes no time).
+ */
+public class MockScheduler implements Scheduler {
+
+ private static class MockTask implements ScheduledFuture<Void> {
+ final String name;
+ final Runnable task;
+ final long period;
+ final Time time;
+
+ private final AtomicLong nextExecution;
Review Comment:
Made this an atomic instead of the inconsistent synchronization that existed before.
--
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 merged pull request #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma merged PR #13092:
URL: https://github.com/apache/kafka/pull/13092
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1064290374
##########
gradle/spotbugs-exclude.xml:
##########
@@ -172,13 +172,6 @@ For a detailed description of spotbugs bug categories, see https://spotbugs.read
<Bug pattern="UMAC_UNCALLABLE_METHOD_OF_ANONYMOUS_CLASS"/>
</Match>
- <Match>
- <!-- Uncallable anonymous methods are left behind after inlining by scalac 2.12, fixed in 2.13 -->
- <Source name="LogConfig.scala"/>
Review Comment:
This is no longer needed since `LogConfig.scala` no longer exists.
--
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 #13092: KAFKA-14607: Move Scheduler/KafkaScheduler to server-common
Posted by GitBox <gi...@apache.org>.
ijuma commented on code in PR #13092:
URL: https://github.com/apache/kafka/pull/13092#discussion_r1065895089
##########
core/src/main/scala/kafka/raft/KafkaMetadataLog.scala:
##########
@@ -674,7 +675,7 @@ object KafkaMetadataLog extends Logging {
logDir: Path,
expiredSnapshots: mutable.TreeMap[OffsetAndEpoch, Option[FileRawSnapshotReader]],
logging: Logging
- ): () => Unit = () => {
+ ): Unit = {
Review Comment:
This made the code less clear, it's better to have the clarity that a lambda is being 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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org