sashapolo commented on code in PR #4900: URL: https://github.com/apache/ignite-3/pull/4900#discussion_r1886394055
########## modules/core/src/main/java/org/apache/ignite/internal/logger/IgniteLoggerImpl.java: ########## @@ -0,0 +1,200 @@ +/* + * 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.ignite.internal.logger; + +import java.lang.System.Logger.Level; +import java.util.Objects; +import java.util.function.Supplier; +import org.apache.ignite.internal.lang.IgniteStringFormatter; +import org.jetbrains.annotations.Nullable; + +class IgniteLoggerImpl implements IgniteLogger { + /** Logger delegate. */ + final System.Logger delegate; + + /** + * Creates logger facade for a given delegate. + * + * @param delegate The delegate to create facade for. + */ + IgniteLoggerImpl(System.Logger delegate) { + this.delegate = delegate; + } + + @Override + public void info(String msg, Object... params) { + logInternal(Level.INFO, msg, null, params); + } + + @Override + public void info(String msg, @Nullable Throwable th, Object... params) { + logInternal(Level.INFO, msg, th, params); + } + + @Override + public void info(Supplier<String> msgSupplier, @Nullable Throwable th) { + logInternalExceptional(Level.INFO, msgSupplier, th); + } + + @Override + public void info(String msg, @Nullable Throwable th) { + delegate.log(Level.INFO, msg, th); + } + + @Override + public void debug(String msg, Object... params) { + logInternal(Level.DEBUG, msg, null, params); + } + + @Override + public void debug(String msg, @Nullable Throwable th, Object... params) { + logInternal(Level.DEBUG, msg, th, params); + } + + @Override + public void debug(Supplier<String> msgSupplier, @Nullable Throwable th) { + logInternalExceptional(Level.DEBUG, msgSupplier, th); + } + + @Override + public void debug(String msg, @Nullable Throwable th) { + delegate.log(Level.DEBUG, msg, th); + } + + @Override + public void warn(String msg, Object... params) { + logInternal(Level.WARNING, msg, null, params); + } + + @Override + public void warn(String msg, @Nullable Throwable th, Object... params) { + logInternal(Level.WARNING, msg, th, params); + } + + @Override + public void warn(Supplier<String> msgSupplier, @Nullable Throwable th) { + logInternalExceptional(Level.WARNING, msgSupplier, th); + } + + @Override + public void warn(String msg, @Nullable Throwable th) { + delegate.log(Level.WARNING, msg, th); + } + + @Override + public void error(String msg, Object... params) { + logInternal(Level.ERROR, msg, null, params); + } + + @Override + public void error(String msg, @Nullable Throwable th, Object... params) { + logInternal(Level.ERROR, msg, th, params); + } + + @Override + public void error(Supplier<String> msgSupplier, @Nullable Throwable th) { + logInternalExceptional(Level.ERROR, msgSupplier, th); + } + + @Override + public void error(String msg, @Nullable Throwable th) { + delegate.log(Level.ERROR, msg, th); + } + + @Override + public void trace(String msg, Object... params) { + logInternal(Level.TRACE, msg, null, params); + } + + @Override + public void trace(String msg, @Nullable Throwable th, Object... params) { + logInternal(Level.TRACE, msg, th, params); + } + + @Override + public void trace(Supplier<String> msgSupplier, @Nullable Throwable th) { + logInternalExceptional(Level.TRACE, msgSupplier, th); + } + + @Override + public void trace(String msg, @Nullable Throwable th) { + delegate.log(Level.TRACE, msg, th); + } + + /** + * Logs a message with an optional list of parameters. + * + * @param level One of the log message level identifiers. Review Comment: Why did you remove parameter alignment from `IgniteLogger`, but kept it here?) ########## modules/core/src/main/java/org/apache/ignite/internal/logger/IgniteLoggerImpl.java: ########## @@ -0,0 +1,200 @@ +/* + * 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.ignite.internal.logger; + +import java.lang.System.Logger.Level; +import java.util.Objects; +import java.util.function.Supplier; +import org.apache.ignite.internal.lang.IgniteStringFormatter; +import org.jetbrains.annotations.Nullable; + +class IgniteLoggerImpl implements IgniteLogger { Review Comment: I would suggest to call this class `IgniteSystemLogger` ########## modules/core/src/main/java/org/apache/ignite/internal/logger/IgniteThrottledLogger.java: ########## @@ -0,0 +1,219 @@ +/* + * 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.ignite.internal.logger; + +import java.lang.System.Logger.Level; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; +import org.jetbrains.annotations.Nullable; + +/** + * {@link IgniteLogger} throttle. + * + * <p>Messages are logged only if they were not logged for the last {@link #THROTTLE_TIMEOUT_MILLIS} milliseconds. Note that not only error + * messages are checked for duplicates, but also exception classes if present.</p> + */ +public interface IgniteThrottledLogger extends IgniteLogger { + /** Throttle timeout in milliseconds (value is 5 min). */ + long THROTTLE_TIMEOUT_MILLIS = TimeUnit.MINUTES.toMillis(5); + + /** + * Logs a message on {@link Level#INFO} level composed from args with given format. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be formatted and passed to the {@link System.Logger}. + * @param params The list of arguments to be substituted in place of formatting anchors. + */ + void info(String throttleKey, String msg, Object... params); + + /** + * Logs a message on {@link Level#INFO} level composed from args with given format and with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be formatted and passed to the {@link System.Logger}. + * @param th The {@code Throwable} associated with log message; can be {@code null}. + * @param params The list of arguments to be substituted in place of formatting anchors. + */ + void info(String throttleKey, String msg, @Nullable Throwable th, Object... params); + + /** + * Logs a message which produces in {@code msgSupplier}, on {@link Level#INFO} level with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msgSupplier A supplier function that produces a message. + * @param th The {@code Throwable} associated with log message; can be {@code null}. + */ + void info(String throttleKey, Supplier<String> msgSupplier, @Nullable Throwable th); + + /** + * Logs a message on {@link Level#INFO} level with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be passed to the {@link System.Logger}. + * @param th The {@code Throwable} associated with the log message. + */ + void info(String throttleKey, String msg, @Nullable Throwable th); + + /** + * Logs a message on {@link Level#DEBUG} level composed from args with given format. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be formatted and passed to the {@link System.Logger}. + * @param params The list of arguments to be substituted in place of formatting anchors. + */ + void debug(String throttleKey, String msg, Object... params); + + /** + * Logs a message on {@link Level#DEBUG} level composed from args with given format and with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be formatted and passed to the {@link System.Logger}. + * @param th The {@code Throwable} associated with log message; can be {@code null}. + * @param params The list of arguments to be substituted in place of formatting anchors. + */ + void debug(String throttleKey, String msg, @Nullable Throwable th, Object... params); + + /** + * Logs a message which produces in {@code msgSupplier}, on {@link Level#DEBUG} level with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msgSupplier A supplier function that produces a message. + * @param th The {@code Throwable} associated with log message; can be {@code null}. + */ + void debug(String throttleKey, Supplier<String> msgSupplier, @Nullable Throwable th); + + /** + * Logs a message on {@link Level#DEBUG} level with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be passed to the {@link System.Logger}. + * @param th The {@code Throwable} associated with the log message; + */ + void debug(String throttleKey, String msg, @Nullable Throwable th); + + /** + * Logs a message on {@link Level#WARNING} level composed from args with given format. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be formatted and passed to the {@link System.Logger}. + * @param params The list of arguments to be substituted in place of formatting anchors. + */ + void warn(String throttleKey, String msg, Object... params); + + /** + * Logs a message on {@link Level#WARNING} level composed from args with given format and with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be formatted and passed to the {@link System.Logger}. + * @param th The {@code Throwable} associated with log message; can be {@code null}. + * @param params The list of arguments to be substituted in place of formatting anchors. + */ + void warn(String throttleKey, String msg, @Nullable Throwable th, Object... params); + + /** + * Logs a message which produces in {@code msgSupplier}, on {@link Level#WARNING} level with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msgSupplier A supplier function that produces a message. + * @param th The {@code Throwable} associated with log message; can be {@code null}. + */ + void warn(String throttleKey, Supplier<String> msgSupplier, @Nullable Throwable th); + + /** + * Logs a message on {@link Level#WARNING} level with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be passed to the {@link System.Logger}. + * @param th The {@code Throwable} associated with the log message. + */ + void warn(String throttleKey, String msg, @Nullable Throwable th); + + /** + * Logs a message on {@link Level#ERROR} level composed from args with given format. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be formatted and passed to the {@link System.Logger}. + * @param params The list of arguments to be substituted in place of formatting anchors. + */ + void error(String throttleKey, String msg, Object... params); + + /** + * Logs a message on {@link Level#ERROR} level composed from args with given format and with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be formatted and passed to the {@link System.Logger}. + * @param th The {@code Throwable} associated with log message; can be {@code null}. + * @param params The list of arguments to be substituted in place of formatting anchors. + */ + void error(String throttleKey, String msg, @Nullable Throwable th, Object... params); + + /** + * Logs a message which produces in {@code msgSupplier}, on {@link Level#ERROR} level with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msgSupplier A supplier function that produces a message. + * @param th The {@code Throwable} associated with log message; can be {@code null}. + */ + void error(String throttleKey, Supplier<String> msgSupplier, @Nullable Throwable th); + + /** + * Logs a message on {@link Level#ERROR} level with associated throwable {@code th}. + * + * @param throttleKey Messages with the same key will be throttled. + * @param msg The message pattern which will be passed to the {@link System.Logger}. + * @param th The {@code Throwable} associated with the log message. + */ + void error(String throttleKey, String msg, @Nullable Throwable th); Review Comment: Do we really need all these methods now? I mean the ones with the `throttleKey`, for example? ########## modules/core/src/main/java/org/apache/ignite/internal/logger/IgniteThrottledLoggerImpl.java: ########## @@ -0,0 +1,359 @@ +/* + * 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.ignite.internal.logger; + +import static org.apache.ignite.internal.util.IgniteUtils.capacity; + +import java.lang.System.Logger; +import java.lang.System.Logger.Level; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; +import org.apache.ignite.internal.lang.IgniteStringFormatter; +import org.apache.ignite.internal.util.FastTimestamps; +import org.jetbrains.annotations.Nullable; + +class IgniteThrottledLoggerImpl implements IgniteThrottledLogger { + /** Logger delegate. */ + private final System.Logger delegate; + + /** Log messages. */ + private final Map<LogThrottleKey, Long> messagesMap = new ConcurrentHashMap<>(capacity(128), 0.75f); Review Comment: I can see that we have `caffeine` in our list of dependencies, maybe we can use it here? It has a configuration that removes entries if they haven't been accessed in a specified duration. ########## modules/core/src/main/java/org/apache/ignite/internal/logger/IgniteThrottledLoggerImpl.java: ########## @@ -0,0 +1,359 @@ +/* + * 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.ignite.internal.logger; + +import static org.apache.ignite.internal.util.IgniteUtils.capacity; + +import java.lang.System.Logger; +import java.lang.System.Logger.Level; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; +import org.apache.ignite.internal.lang.IgniteStringFormatter; +import org.apache.ignite.internal.util.FastTimestamps; +import org.jetbrains.annotations.Nullable; + +class IgniteThrottledLoggerImpl implements IgniteThrottledLogger { + /** Logger delegate. */ + private final System.Logger delegate; + + /** Log messages. */ + private final Map<LogThrottleKey, Long> messagesMap = new ConcurrentHashMap<>(capacity(128), 0.75f); Review Comment: why do you explicitly specify the load factor? ########## modules/core/src/main/java/org/apache/ignite/internal/logger/IgniteThrottledLoggerImpl.java: ########## @@ -0,0 +1,359 @@ +/* + * 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.ignite.internal.logger; + +import static org.apache.ignite.internal.util.IgniteUtils.capacity; + +import java.lang.System.Logger; +import java.lang.System.Logger.Level; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; +import org.apache.ignite.internal.lang.IgniteStringFormatter; +import org.apache.ignite.internal.util.FastTimestamps; +import org.jetbrains.annotations.Nullable; + +class IgniteThrottledLoggerImpl implements IgniteThrottledLogger { + /** Logger delegate. */ + private final System.Logger delegate; + + /** Log messages. */ + private final Map<LogThrottleKey, Long> messagesMap = new ConcurrentHashMap<>(capacity(128), 0.75f); + + IgniteThrottledLoggerImpl(Logger delegate, ScheduledExecutorService scheduledExecutor, ExecutorService executor) { Review Comment: Do we really need two executors here? What's wrong with clearing messages inside `scheduledExecutor`? -- 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: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
