This is an automated email from the ASF dual-hosted git repository.
sergeychugunov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new aebe8ab IGNITE-15416 False warnings from default Checkpoint and
Collision SPIs
aebe8ab is described below
commit aebe8ab29e58b4b36dda1e069958fae3eeb17577
Author: Roman Puchkovskiy <[email protected]>
AuthorDate: Fri Oct 22 11:02:00 2021 +0300
IGNITE-15416 False warnings from default Checkpoint and Collision SPIs
- The following warning was written to log when starting with the default
CheckpointSpi (NoOpCheckpointSpi):
WARNING: Checkpoints are disabled (to enable configure any
GridCheckpointSpi implementation)
This happens even if the user does not use checkpointing at all. In such a
case, we should not log this message, but instead it was logged as a WARNING
which was confusing.
The change is that now we log it only on the first attempt to actually
save/load/remove a checkpoint, not at startup time.
- The following was written to log when starting without collision
resolution configuration:
WARNING: Collision resolution is disabled (all jobs will be activated upon
arrival).
This is the normal mode, and it should not be a warning.
The change is to log it at INFO level. - Fixes #9503.
Signed-off-by: Sergey Chugunov <[email protected]>
---
.../managers/collision/GridCollisionManager.java | 4 +-
.../spi/checkpoint/noop/NoopCheckpointSpi.java | 18 ++-
.../GridCollisionManagerLoggingSelfTest.java | 74 ++++++++++
.../noop/NoopCheckpointSpiLoggingTest.java | 152 +++++++++++++++++++++
.../ignite/testsuites/IgniteBasicTestSuite.java | 5 +-
.../testsuites/IgniteComputeGridTestSuite.java | 2 +
6 files changed, 251 insertions(+), 4 deletions(-)
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/managers/collision/GridCollisionManager.java
b/modules/core/src/main/java/org/apache/ignite/internal/managers/collision/GridCollisionManager.java
index a7626bf..2282e60 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/managers/collision/GridCollisionManager.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/managers/collision/GridCollisionManager.java
@@ -19,11 +19,11 @@ package org.apache.ignite.internal.managers.collision;
import java.util.Collection;
import java.util.concurrent.atomic.AtomicReference;
+
import org.apache.ignite.IgniteCheckedException;
import org.apache.ignite.internal.GridKernalContext;
import org.apache.ignite.internal.SkipDaemon;
import org.apache.ignite.internal.managers.GridManagerAdapter;
-import org.apache.ignite.internal.util.typedef.internal.U;
import org.apache.ignite.spi.collision.CollisionContext;
import org.apache.ignite.spi.collision.CollisionExternalListener;
import org.apache.ignite.spi.collision.CollisionJobContext;
@@ -61,7 +61,7 @@ public class GridCollisionManager extends
GridManagerAdapter<CollisionSpi> {
});
}
else
- U.warn(log, "Collision resolution is disabled (all jobs will be
activated upon arrival).");
+ log.info("Collision resolution is disabled (all jobs will be
activated upon arrival).");
if (log.isDebugEnabled())
log.debug(startInfo());
diff --git
a/modules/core/src/main/java/org/apache/ignite/spi/checkpoint/noop/NoopCheckpointSpi.java
b/modules/core/src/main/java/org/apache/ignite/spi/checkpoint/noop/NoopCheckpointSpi.java
index eb24926..52f530f 100644
---
a/modules/core/src/main/java/org/apache/ignite/spi/checkpoint/noop/NoopCheckpointSpi.java
+++
b/modules/core/src/main/java/org/apache/ignite/spi/checkpoint/noop/NoopCheckpointSpi.java
@@ -17,6 +17,8 @@
package org.apache.ignite.spi.checkpoint.noop;
+import java.util.concurrent.atomic.AtomicBoolean;
+
import org.apache.ignite.IgniteLogger;
import org.apache.ignite.internal.util.typedef.internal.S;
import org.apache.ignite.internal.util.typedef.internal.U;
@@ -40,9 +42,12 @@ public class NoopCheckpointSpi extends IgniteSpiAdapter
implements CheckpointSpi
@LoggerResource
private IgniteLogger log;
+ /** Whether a warning about disabled checkpoints was already logged. */
+ private final AtomicBoolean warnedAboutDisabledCheckpoints = new
AtomicBoolean(false);
+
/** {@inheritDoc} */
@Override public void spiStart(@Nullable String igniteInstanceName) throws
IgniteSpiException {
- U.warn(log, "Checkpoints are disabled (to enable configure any
GridCheckpointSpi implementation)");
+ // No-op.
}
/** {@inheritDoc} */
@@ -52,16 +57,19 @@ public class NoopCheckpointSpi extends IgniteSpiAdapter
implements CheckpointSpi
/** {@inheritDoc} */
@Nullable @Override public byte[] loadCheckpoint(String key) throws
IgniteSpiException {
+ warnOnceAboutDisabledCheckpoints();
return null;
}
/** {@inheritDoc} */
@Override public boolean saveCheckpoint(String key, byte[] state, long
timeout, boolean overwrite) {
+ warnOnceAboutDisabledCheckpoints();
return false;
}
/** {@inheritDoc} */
@Override public boolean removeCheckpoint(String key) {
+ warnOnceAboutDisabledCheckpoints();
return false;
}
@@ -77,6 +85,14 @@ public class NoopCheckpointSpi extends IgniteSpiAdapter
implements CheckpointSpi
return this;
}
+ /**
+ * Logs a warning that checkpoints are disabled. Only does so on first
invocation, subsequent ones are ignored.
+ */
+ private void warnOnceAboutDisabledCheckpoints() {
+ if (warnedAboutDisabledCheckpoints.compareAndSet(false, true))
+ U.warn(log, "Checkpoints are disabled (to enable configure any
GridCheckpointSpi implementation)");
+ }
+
/** {@inheritDoc} */
@Override public String toString() {
return S.toString(NoopCheckpointSpi.class, this);
diff --git
a/modules/core/src/test/java/org/apache/ignite/internal/GridCollisionManagerLoggingSelfTest.java
b/modules/core/src/test/java/org/apache/ignite/internal/GridCollisionManagerLoggingSelfTest.java
new file mode 100644
index 0000000..f66a1da
--- /dev/null
+++
b/modules/core/src/test/java/org/apache/ignite/internal/GridCollisionManagerLoggingSelfTest.java
@@ -0,0 +1,74 @@
+/*
+ * 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;
+
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.internal.managers.collision.GridCollisionManager;
+import org.apache.ignite.spi.collision.noop.NoopCollisionSpi;
+import org.apache.ignite.testframework.junits.GridTestKernalContext;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Tests for making sure that {@link GridCollisionManager} logs about specific
conditions at correct levels,
+ * messages are correct and so on.
+ */
+public class GridCollisionManagerLoggingSelfTest {
+ /** Logger used to intercept and inspect logged messages. */
+ private final IgniteLogger logger = mock(IgniteLogger.class);
+
+ /**
+ * Inits logger mock to make it behave nicely (like return a non-null
logger from
+ * <tt>IgniteLogger#getLogger()</tt> method).
+ */
+ @Before
+ public void initLoggerMock() {
+ doReturn(logger).when(logger).getLogger(any());
+ }
+
+ /**
+ * Makes sure that, given collision resolution is disabled, during start
up the corresponding
+ * message is logged as INFO, not as WARN.
+ *
+ * @throws Exception if something goes wrong
+ */
+ @Test
+ public void collisionResolutionDisabledMessageShouldBeLoggedAtInfoLevel()
throws Exception {
+ GridCollisionManager manager = new
GridCollisionManager(collisionResolutionDisabledContext());
+
+ manager.start();
+
+ verify(logger).info("Collision resolution is disabled (all jobs will
be activated upon arrival).");
+ }
+
+ /**
+ * Builds context without collision resolution.
+ *
+ * @return context without collision resolution
+ */
+ private GridTestKernalContext collisionResolutionDisabledContext() {
+ GridTestKernalContext context = new GridTestKernalContext(logger);
+ context.config().setCollisionSpi(new NoopCollisionSpi());
+ return context;
+ }
+}
diff --git
a/modules/core/src/test/java/org/apache/ignite/spi/checkpoint/noop/NoopCheckpointSpiLoggingTest.java
b/modules/core/src/test/java/org/apache/ignite/spi/checkpoint/noop/NoopCheckpointSpiLoggingTest.java
new file mode 100644
index 0000000..c7f3153
--- /dev/null
+++
b/modules/core/src/test/java/org/apache/ignite/spi/checkpoint/noop/NoopCheckpointSpiLoggingTest.java
@@ -0,0 +1,152 @@
+/*
+ * 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.spi.checkpoint.noop;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.regex.Pattern;
+
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.testframework.ListeningTestLogger;
+import org.apache.ignite.testframework.LogListener;
+import org.jetbrains.annotations.NotNull;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Tests for logging concerns of {@link NoopCheckpointSpi}: how and when it
logs or does not log.
+ */
+public class NoopCheckpointSpiLoggingTest {
+ /** An empty byte array. */
+ private static final byte[] EMPTY_ARRAY = new byte[0];
+
+ /** The tested SPI instance. */
+ private final NoopCheckpointSpi checkpointSpi = new NoopCheckpointSpi();
+
+ /** Logger used to intercept and inspect logged messages. */
+ private final ListeningTestLogger logger = new ListeningTestLogger();
+
+ /** Logging listener used throught the test. */
+ private LogListener listener;
+
+ /**
+ * Injects logger into SPI instance to prepare it for work.
+ *
+ * @throws Exception if something goes wrong
+ */
+ @Before
+ public void injectLogger() throws Exception {
+ for (Field field : NoopCheckpointSpi.class.getDeclaredFields()) {
+ if (!Modifier.isStatic(field.getModifiers()) && field.getType() ==
IgniteLogger.class) {
+ field.setAccessible(true);
+ field.set(checkpointSpi, logger);
+ }
+ }
+ }
+
+ /**
+ * Makes sure that 'checkpoints are disabled' is not logged at startup.
+ */
+ @Test
+ public void shouldNotLogAboutCheckpointsBeingDisabledAtStartup() {
+ LogListener listener =
LogListener.matches(Pattern.compile("Checkpoints are disabled.+"))
+ .atMost(0)
+ .build();
+ logger.registerListener(listener);
+
+ checkpointSpi.spiStart("test");
+
+ assertTrue(listener.check());
+ }
+
+ /**
+ * Makes sure that 'checkpoints are disabled' is logged at a first attempt
to save a checkoint.
+ */
+ @Test
+ public void shouldLogAboutCheckpointsBeingDisabledOnCheckpointSave() {
+ registerListenerExpectingExactlyOneCheckpointsDisabledMessage();
+
+ checkpointSpi.saveCheckpoint("point", EMPTY_ARRAY, 1, true);
+
+ verifyExactlyOneLogMessageSeen();
+ }
+
+ /**
+ * Makes sure that exactly one expected message is seen by the logger.
+ */
+ private void verifyExactlyOneLogMessageSeen() {
+ assertTrue(listener.check());
+ }
+
+ /**
+ * Registers a listener for 'checkpoints are disabled' message that
expects exactly one such message.
+ */
+ @NotNull
+ private LogListener
registerListenerExpectingExactlyOneCheckpointsDisabledMessage() {
+ listener = LogListener
+ .matches("Checkpoints are disabled (to enable configure any
GridCheckpointSpi implementation)")
+ .times(1)
+ .build();
+ logger.registerListener(listener);
+ return listener;
+ }
+
+ /**
+ * Makes sure that 'checkpoints are disabled' is logged at a first attempt
to load a checkoint.
+ */
+ @Test
+ public void shouldLogAboutCheckpointsBeingDisabledOnCheckpointLoad() {
+ registerListenerExpectingExactlyOneCheckpointsDisabledMessage();
+
+ checkpointSpi.loadCheckpoint("point");
+
+ verifyExactlyOneLogMessageSeen();
+ }
+
+ /**
+ * Makes sure that 'checkpoints are disabled' is logged at a first attempt
to remove a checkoint.
+ */
+ @Test
+ public void shouldLogAboutCheckpointsBeingDisabledOnCheckpointRemoval() {
+ registerListenerExpectingExactlyOneCheckpointsDisabledMessage();
+
+ checkpointSpi.removeCheckpoint("point");
+
+ verifyExactlyOneLogMessageSeen();
+ }
+
+ /**
+ * Makes sure that 'checkpoints are disabled' is logged at most once.
+ */
+ @Test
+ public void shouldOnlyLogOnceAboutCheckointsBeingDisabled() {
+ registerListenerExpectingExactlyOneCheckpointsDisabledMessage();
+
+ // invoke every potentially logging method twice
+ checkpointSpi.saveCheckpoint("point", EMPTY_ARRAY, 1, true);
+ checkpointSpi.saveCheckpoint("point", EMPTY_ARRAY, 1, true);
+ checkpointSpi.loadCheckpoint("point");
+ checkpointSpi.loadCheckpoint("point");
+ checkpointSpi.removeCheckpoint("point");
+ checkpointSpi.removeCheckpoint("point");
+
+ verifyExactlyOneLogMessageSeen();
+ }
+}
diff --git
a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicTestSuite.java
b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicTestSuite.java
index 4039380..411fdd5 100644
---
a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicTestSuite.java
+++
b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicTestSuite.java
@@ -131,6 +131,7 @@ import org.apache.ignite.plugin.PluginConfigurationTest;
import org.apache.ignite.plugin.PluginNodeValidationTest;
import org.apache.ignite.plugin.security.SecurityPermissionSetBuilderTest;
import org.apache.ignite.spi.GridSpiLocalHostInjectionTest;
+import org.apache.ignite.spi.checkpoint.noop.NoopCheckpointSpiLoggingTest;
import org.apache.ignite.startup.properties.NotStringSystemPropertyTest;
import org.apache.ignite.testframework.MessageOrderLogListenerTest;
import org.apache.ignite.testframework.test.ConfigVariationsExecutionTest;
@@ -322,7 +323,9 @@ import org.junit.runners.Suite;
// Other tests
CacheLockCandidatesThreadTest.class,
- RemoveAllDeadlockTest.class
+ RemoveAllDeadlockTest.class,
+
+ NoopCheckpointSpiLoggingTest.class
})
public class IgniteBasicTestSuite {
}
diff --git
a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteComputeGridTestSuite.java
b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteComputeGridTestSuite.java
index f63dfd5..61cc2a4 100644
---
a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteComputeGridTestSuite.java
+++
b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteComputeGridTestSuite.java
@@ -27,6 +27,7 @@ import
org.apache.ignite.internal.GridCancelOnGridStopSelfTest;
import org.apache.ignite.internal.GridCancelUnusedJobSelfTest;
import org.apache.ignite.internal.GridCancelledJobsMetricsSelfTest;
import org.apache.ignite.internal.GridCollisionJobsContextSelfTest;
+import org.apache.ignite.internal.GridCollisionManagerLoggingSelfTest;
import org.apache.ignite.internal.GridDeploymentMultiThreadedSelfTest;
import org.apache.ignite.internal.GridDeploymentSelfTest;
import org.apache.ignite.internal.GridEventStorageCheckAllEventsSelfTest;
@@ -164,6 +165,7 @@ import org.junit.runners.Suite;
PublicThreadpoolStarvationTest.class,
StripedExecutorTest.class,
GridJobServicesAddNodeTest.class,
+ GridCollisionManagerLoggingSelfTest.class,
IgniteComputeCustomExecutorConfigurationSelfTest.class,
IgniteComputeCustomExecutorSelfTest.class,
