You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "tmgodinho (via GitHub)" <gi...@apache.org> on 2024/05/14 17:08:39 UTC
[PR] Ignite-22242: Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
tmgodinho opened a new pull request, #3757:
URL: https://github.com/apache/ignite-3/pull/3757
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
### The Review Checklist
- [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:
\- There is a single JIRA ticket related to the pull request.
\- The web-link to the pull request is attached to the JIRA ticket.
\- The JIRA ticket has the Patch Available state.
\- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
\- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
- [ ] **Design:** new code conforms with the design principles of the components it is added to.
- [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
- [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
- [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
### Notes
- [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "tmgodinho (via GitHub)" <gi...@apache.org>.
tmgodinho commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1608361905
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/LogStorageFactory.java:
##########
@@ -43,4 +47,16 @@ public interface LogStorageFactory extends ManuallyCloseable, LogSyncer {
*/
@Override
void close();
+
+ @Override
+ default CompletableFuture<Void> startAsync() {
+ start();
+ return nullCompletedFuture();
+ }
+
+ @Override
+ default CompletableFuture<Void> stopAsync() {
+ close();
+ return nullCompletedFuture();
+ }
Review Comment:
Disregard my last comment, @sashapolo had already answered
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1609601975
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/DefaultLogStorageFactory.java:
##########
@@ -105,9 +108,18 @@ public DefaultLogStorageFactory(String nodeName, Supplier<Path> logPathSupplier)
);
}
- /** {@inheritDoc} */
@Override
- public void start() {
+ public CompletableFuture<Void> startAsync() {
+ // This is effectively a sync implementation.
+ try {
+ start();
+ return nullCompletedFuture();
+ } catch (RuntimeException ex) {
+ return failedFuture(ex);
+ }
+ }
+
Review Comment:
> Do you think we need to create a new ticket to make this full async in the future?
No, I think that's by design
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1608287243
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/MockitoTestUtils.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.testframework;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.withSettings;
+
+import java.util.function.Supplier;
+import org.mockito.Answers;
+import org.mockito.invocation.InvocationOnMock;
+
+/** Some utilities for tests that use Mockito. */
+public class MockitoTestUtils {
Review Comment:
Ι'm glad to hear it.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "tmgodinho (via GitHub)" <gi...@apache.org>.
tmgodinho commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1609565382
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/DefaultLogStorageFactory.java:
##########
@@ -105,9 +108,18 @@ public DefaultLogStorageFactory(String nodeName, Supplier<Path> logPathSupplier)
);
}
- /** {@inheritDoc} */
@Override
- public void start() {
+ public CompletableFuture<Void> startAsync() {
+ // This is effectively a sync implementation.
+ try {
+ start();
+ return nullCompletedFuture();
+ } catch (RuntimeException ex) {
+ return failedFuture(ex);
+ }
+ }
+
Review Comment:
Yes, that's correct. Other implementations also kept a sync implementation after the refactoring.
I intended to write a comment in this line but eventually forgot about it.
Do you think we need to create a new ticket to make this full async in the future? and link it here? Or this is so common that is not necessary.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1609330592
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/DefaultLogStorageFactory.java:
##########
@@ -105,9 +108,18 @@ public DefaultLogStorageFactory(String nodeName, Supplier<Path> logPathSupplier)
);
}
- /** {@inheritDoc} */
@Override
- public void start() {
+ public CompletableFuture<Void> startAsync() {
+ // This is effectively a sync implementation.
+ try {
+ start();
+ return nullCompletedFuture();
+ } catch (RuntimeException ex) {
+ return failedFuture(ex);
+ }
+ }
+
Review Comment:
By meaning of startAsync -- start() looks like should be started in future: `CompletableFuture.runAsync(() -> start());` The only question is: in most cases a component's executor should be provided.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "tmgodinho (via GitHub)" <gi...@apache.org>.
tmgodinho commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1607875566
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/util/SharedLogStorageFactoryUtils.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.raft.util;
+
+import static org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
+
+import java.nio.file.Path;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.BiFunction;
+import java.util.function.Supplier;
+import org.apache.ignite.internal.lang.IgniteSystemProperties;
+import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.raft.configuration.RaftConfiguration;
+import org.apache.ignite.internal.raft.storage.LogStorageFactory;
+import org.apache.ignite.internal.raft.storage.impl.DefaultLogStorageFactory;
+import org.apache.ignite.internal.raft.storage.logit.LogitLogStorageFactory;
+import org.apache.ignite.raft.jraft.storage.logit.option.StoreOptions;
+
+/** Utility methods for creating {@link LogStorageFactory}is for the Shared Log. */
+public class SharedLogStorageFactoryUtils {
+ /**
+ * Enables logit log storage. {@code false} by default.
+ * This is a temporary property, that should only be used for testing and comparing the two storages.
+ */
+ public static final String LOGIT_STORAGE_ENABLED_PROPERTY = "LOGIT_STORAGE_ENABLED";
+
+ /** Creates a LogStorageFactory with the {@link DefaultLogStorageFactory} implementation. */
+ public static LogStorageFactory create(String nodeName, Path workDir, RaftConfiguration raftConfiguration) {
+ return create(nodeName, workDir, raftConfiguration, DefaultLogStorageFactory::new);
+ }
+
+ /** Creates a LogStorageFactory with the provided factory or the {@link LogitLogStorageFactory} implementation. */
+ public static LogStorageFactory create(
+ String nodeName,
+ Path workDir,
+ RaftConfiguration raftConfiguration,
+ BiFunction<String, Supplier<Path>, LogStorageFactory> baseFactory
+ ) {
+ Supplier<Path> logStoragePath = () -> raftConfiguration.logPath().value().isEmpty()
+ ? workDir.resolve("log")
+ : Path.of(raftConfiguration.logPath().value());
+
+ return IgniteSystemProperties.getBoolean(LOGIT_STORAGE_ENABLED_PROPERTY, false)
+ ? new LogitLogStorageFactory(nodeName, new StoreOptions(), logStoragePath)
+ : baseFactory.apply(nodeName, logStoragePath);
+ }
+
+ /** Wraps the LogStorageFactory in a IgniteComponent. */
+ public static IgniteComponent wrapWithComponent(LogStorageFactory logStorageFactory) {
Review Comment:
It was an already existing interface but it is ignite code
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "tmgodinho (via GitHub)" <gi...@apache.org>.
tmgodinho commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1607959940
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/MockitoTestUtils.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.testframework;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.withSettings;
+
+import java.util.function.Supplier;
+import org.mockito.Answers;
+import org.mockito.invocation.InvocationOnMock;
+
+/** Some utilities for tests that use Mockito. */
+public class MockitoTestUtils {
+ private MockitoTestUtils() {
+ // Intentionally left blank.
+ }
+
+ /**
+ * Creates a mockito stub only spy. Similar to {@link org.mockito.Mockito#spy(Object)} but does not track invocations.
+ * Useful to add append functionality to existing methods.
+ *
+ * @param <T> The type of the spied object.
+ * @param spiedInstanceSupplier Supplier for the spied instance. Removes the need for temporary variables.
+ * @return The created mockito spy.
+ */
+ public static <T> T spyStubOnly(Supplier<T> spiedInstanceSupplier) {
+ T instance = spiedInstanceSupplier.get();
Review Comment:
Removed.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "tmgodinho (via GitHub)" <gi...@apache.org>.
tmgodinho commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1608343256
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/LogStorageFactory.java:
##########
@@ -43,4 +47,16 @@ public interface LogStorageFactory extends ManuallyCloseable, LogSyncer {
*/
@Override
void close();
+
+ @Override
+ default CompletableFuture<Void> startAsync() {
+ start();
+ return nullCompletedFuture();
+ }
+
+ @Override
+ default CompletableFuture<Void> stopAsync() {
+ close();
+ return nullCompletedFuture();
+ }
Review Comment:
My reasoning was just to adapt the start/stop methods to the IgniteComponent interface.
The main difference for these methods is that the IgniteComponent are async and the native start and stop of this class are sync.
@JAkutenshi and @sashapolo do you think it's worth removing the original start/stop methods and changing the implementations to comply with the IgniteComponent interface??
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1607127796
##########
modules/table/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java:
##########
@@ -1320,6 +1328,7 @@ void start() {
nodeCfgMgr,
failureProcessor,
clusterService,
+ logStorageFactoryComponent, // Check when to stop!!
Review Comment:
?
##########
modules/raft/src/testFixtures/java/org/apache/ignite/internal/raft/LozaUtils.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.raft;
+
+import java.nio.file.Path;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.internal.hlc.HybridClock;
+import org.apache.ignite.internal.metrics.NoOpMetricManager;
+import org.apache.ignite.internal.network.ClusterService;
+import org.apache.ignite.internal.raft.configuration.RaftConfiguration;
+import org.apache.ignite.internal.raft.storage.LogStorageFactory;
+import org.apache.ignite.internal.raft.util.SharedLogStorageFactoryUtils;
+import org.apache.ignite.raft.jraft.rpc.impl.RaftGroupEventsClientListener;
+
+/** Utilities for creating {@link Loza} instances. */
+public class LozaUtils {
Review Comment:
I would call this class `TestLozaFactory`, what do you think?
##########
modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/ItSimpleCounterServerTest.java:
##########
@@ -99,12 +101,8 @@ void before() throws Exception {
ClusterService service = clusterService(PORT, List.of(addr), true);
- server = new JraftServerImpl(service, workDir, raftConfiguration) {
- @Override
- public CompletableFuture<Void> stopAsync() {
- return IgniteUtils.stopAsync(super::stopAsync, service::stopAsync);
- }
- };
+ server = MockitoTestUtils.spyStubOnly(() -> JraftServerUtils.create(service, workDir, raftConfiguration));
+ doAnswer(ans -> IgniteUtils.stopAsync(() -> tryCallRealMethod(ans), service::stopAsync)).when(server).stopAsync();
Review Comment:
I think we need to refactor the test to explicitly close the `service`. This approach with mocks looks like a hack.
##########
modules/raft/src/testFixtures/java/org/apache/ignite/internal/raft/server/JraftServerUtils.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.raft.server;
+
+import java.nio.file.Path;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.internal.network.ClusterService;
+import org.apache.ignite.internal.raft.configuration.RaftConfiguration;
+import org.apache.ignite.internal.raft.server.impl.JraftServerImpl;
+import org.apache.ignite.internal.raft.storage.LogStorageFactory;
+import org.apache.ignite.internal.raft.util.SharedLogStorageFactoryUtils;
+import org.apache.ignite.raft.jraft.option.NodeOptions;
+import org.apache.ignite.raft.jraft.rpc.impl.RaftGroupEventsClientListener;
+
+/** Utilities for creating JRaftServer instances. */
+public class JraftServerUtils {
Review Comment:
Same here: `TestJraftServerFactory`
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/util/SharedLogStorageFactoryUtils.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.raft.util;
+
+import static org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
+
+import java.nio.file.Path;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.BiFunction;
+import java.util.function.Supplier;
+import org.apache.ignite.internal.lang.IgniteSystemProperties;
+import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.raft.configuration.RaftConfiguration;
+import org.apache.ignite.internal.raft.storage.LogStorageFactory;
+import org.apache.ignite.internal.raft.storage.impl.DefaultLogStorageFactory;
+import org.apache.ignite.internal.raft.storage.logit.LogitLogStorageFactory;
+import org.apache.ignite.raft.jraft.storage.logit.option.StoreOptions;
+
+/** Utility methods for creating {@link LogStorageFactory}is for the Shared Log. */
+public class SharedLogStorageFactoryUtils {
+ /**
+ * Enables logit log storage. {@code false} by default.
+ * This is a temporary property, that should only be used for testing and comparing the two storages.
+ */
+ public static final String LOGIT_STORAGE_ENABLED_PROPERTY = "LOGIT_STORAGE_ENABLED";
+
+ /** Creates a LogStorageFactory with the {@link DefaultLogStorageFactory} implementation. */
+ public static LogStorageFactory create(String nodeName, Path workDir, RaftConfiguration raftConfiguration) {
+ return create(nodeName, workDir, raftConfiguration, DefaultLogStorageFactory::new);
+ }
+
+ /** Creates a LogStorageFactory with the provided factory or the {@link LogitLogStorageFactory} implementation. */
+ public static LogStorageFactory create(
+ String nodeName,
+ Path workDir,
+ RaftConfiguration raftConfiguration,
+ BiFunction<String, Supplier<Path>, LogStorageFactory> baseFactory
+ ) {
+ Supplier<Path> logStoragePath = () -> raftConfiguration.logPath().value().isEmpty()
+ ? workDir.resolve("log")
+ : Path.of(raftConfiguration.logPath().value());
+
+ return IgniteSystemProperties.getBoolean(LOGIT_STORAGE_ENABLED_PROPERTY, false)
+ ? new LogitLogStorageFactory(nodeName, new StoreOptions(), logStoragePath)
+ : baseFactory.apply(nodeName, logStoragePath);
+ }
+
+ /** Wraps the LogStorageFactory in a IgniteComponent. */
+ public static IgniteComponent wrapWithComponent(LogStorageFactory logStorageFactory) {
Review Comment:
Is `LogStorageFactory` a class created by us? Can we simply make it an `IgniteComponent` straight away?
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/MockitoTestUtils.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.testframework;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.withSettings;
+
+import java.util.function.Supplier;
+import org.mockito.Answers;
+import org.mockito.invocation.InvocationOnMock;
+
+/** Some utilities for tests that use Mockito. */
+public class MockitoTestUtils {
+ private MockitoTestUtils() {
+ // Intentionally left blank.
+ }
+
+ /**
+ * Creates a mockito stub only spy. Similar to {@link org.mockito.Mockito#spy(Object)} but does not track invocations.
+ * Useful to add append functionality to existing methods.
+ *
+ * @param <T> The type of the spied object.
+ * @param spiedInstanceSupplier Supplier for the spied instance. Removes the need for temporary variables.
+ * @return The created mockito spy.
+ */
+ public static <T> T spyStubOnly(Supplier<T> spiedInstanceSupplier) {
+ T instance = spiedInstanceSupplier.get();
Review Comment:
I don't understand how this `Supplier` helps with eliminating temporary variables. What is the difference, for example, between
```
MockitoTestUtils.spyStubOnly(() -> JraftServerUtils.create(service, workDir, raftConfiguration));
```
and
```
MockitoTestUtils.spyStubOnly(JraftServerUtils.create(service, workDir, raftConfiguration));
```
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/MockitoTestUtils.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.testframework;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.withSettings;
+
+import java.util.function.Supplier;
+import org.mockito.Answers;
+import org.mockito.invocation.InvocationOnMock;
+
+/** Some utilities for tests that use Mockito. */
+public class MockitoTestUtils {
Review Comment:
I would really prefer refactoring the test code instead of introducing this 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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1607883249
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/util/SharedLogStorageFactoryUtils.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.raft.util;
+
+import static org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
+
+import java.nio.file.Path;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.BiFunction;
+import java.util.function.Supplier;
+import org.apache.ignite.internal.lang.IgniteSystemProperties;
+import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.raft.configuration.RaftConfiguration;
+import org.apache.ignite.internal.raft.storage.LogStorageFactory;
+import org.apache.ignite.internal.raft.storage.impl.DefaultLogStorageFactory;
+import org.apache.ignite.internal.raft.storage.logit.LogitLogStorageFactory;
+import org.apache.ignite.raft.jraft.storage.logit.option.StoreOptions;
+
+/** Utility methods for creating {@link LogStorageFactory}is for the Shared Log. */
+public class SharedLogStorageFactoryUtils {
+ /**
+ * Enables logit log storage. {@code false} by default.
+ * This is a temporary property, that should only be used for testing and comparing the two storages.
+ */
+ public static final String LOGIT_STORAGE_ENABLED_PROPERTY = "LOGIT_STORAGE_ENABLED";
+
+ /** Creates a LogStorageFactory with the {@link DefaultLogStorageFactory} implementation. */
+ public static LogStorageFactory create(String nodeName, Path workDir, RaftConfiguration raftConfiguration) {
+ return create(nodeName, workDir, raftConfiguration, DefaultLogStorageFactory::new);
+ }
+
+ /** Creates a LogStorageFactory with the provided factory or the {@link LogitLogStorageFactory} implementation. */
+ public static LogStorageFactory create(
+ String nodeName,
+ Path workDir,
+ RaftConfiguration raftConfiguration,
+ BiFunction<String, Supplier<Path>, LogStorageFactory> baseFactory
+ ) {
+ Supplier<Path> logStoragePath = () -> raftConfiguration.logPath().value().isEmpty()
+ ? workDir.resolve("log")
+ : Path.of(raftConfiguration.logPath().value());
+
+ return IgniteSystemProperties.getBoolean(LOGIT_STORAGE_ENABLED_PROPERTY, false)
+ ? new LogitLogStorageFactory(nodeName, new StoreOptions(), logStoragePath)
+ : baseFactory.apply(nodeName, logStoragePath);
+ }
+
+ /** Wraps the LogStorageFactory in a IgniteComponent. */
+ public static IgniteComponent wrapWithComponent(LogStorageFactory logStorageFactory) {
Review Comment:
What I meant is: did this interface come from JRaft code? If not, it was created by us and we can change it.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "tmgodinho (via GitHub)" <gi...@apache.org>.
tmgodinho commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1607959378
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/MockitoTestUtils.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.testframework;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.withSettings;
+
+import java.util.function.Supplier;
+import org.mockito.Answers;
+import org.mockito.invocation.InvocationOnMock;
+
+/** Some utilities for tests that use Mockito. */
+public class MockitoTestUtils {
Review Comment:
It was actually not that hard. Most of the tests were not using that functionality at all.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "tmgodinho (via GitHub)" <gi...@apache.org>.
tmgodinho commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1608339264
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/util/SharedLogStorageFactoryUtils.java:
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.raft.util;
+
+import static org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
+
+import java.nio.file.Path;
+import java.util.concurrent.CompletableFuture;
+import java.util.function.BiFunction;
+import java.util.function.Supplier;
+import org.apache.ignite.internal.lang.IgniteSystemProperties;
+import org.apache.ignite.internal.manager.IgniteComponent;
+import org.apache.ignite.internal.raft.configuration.RaftConfiguration;
+import org.apache.ignite.internal.raft.storage.LogStorageFactory;
+import org.apache.ignite.internal.raft.storage.impl.DefaultLogStorageFactory;
+import org.apache.ignite.internal.raft.storage.logit.LogitLogStorageFactory;
+import org.apache.ignite.raft.jraft.storage.logit.option.StoreOptions;
+
+/** Utility methods for creating {@link LogStorageFactory}is for the Shared Log. */
+public class SharedLogStorageFactoryUtils {
+ /**
+ * Enables logit log storage. {@code false} by default.
+ * This is a temporary property, that should only be used for testing and comparing the two storages.
+ */
+ public static final String LOGIT_STORAGE_ENABLED_PROPERTY = "LOGIT_STORAGE_ENABLED";
+
+ /** Creates a LogStorageFactory with the {@link DefaultLogStorageFactory} implementation. */
+ public static LogStorageFactory create(String nodeName, Path workDir, RaftConfiguration raftConfiguration) {
+ return create(nodeName, workDir, raftConfiguration, DefaultLogStorageFactory::new);
+ }
+
+ /** Creates a LogStorageFactory with the provided factory or the {@link LogitLogStorageFactory} implementation. */
+ public static LogStorageFactory create(
+ String nodeName,
+ Path workDir,
+ RaftConfiguration raftConfiguration,
+ BiFunction<String, Supplier<Path>, LogStorageFactory> baseFactory
+ ) {
+ Supplier<Path> logStoragePath = () -> raftConfiguration.logPath().value().isEmpty()
+ ? workDir.resolve("log")
+ : Path.of(raftConfiguration.logPath().value());
+
+ return IgniteSystemProperties.getBoolean(LOGIT_STORAGE_ENABLED_PROPERTY, false)
+ ? new LogitLogStorageFactory(nodeName, new StoreOptions(), logStoragePath)
+ : baseFactory.apply(nodeName, logStoragePath);
+ }
+
+ /** Wraps the LogStorageFactory in a IgniteComponent. */
+ public static IgniteComponent wrapWithComponent(LogStorageFactory logStorageFactory) {
Review Comment:
No, I don't think it came from JRaft. The JRaft fork commit seems to pre-date this 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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1609777163
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/DefaultLogStorageFactory.java:
##########
@@ -105,9 +108,18 @@ public DefaultLogStorageFactory(String nodeName, Supplier<Path> logPathSupplier)
);
}
- /** {@inheritDoc} */
@Override
- public void start() {
+ public CompletableFuture<Void> startAsync() {
+ // This is effectively a sync implementation.
+ try {
+ start();
+ return nullCompletedFuture();
+ } catch (RuntimeException ex) {
+ return failedFuture(ex);
+ }
+ }
+
Review Comment:
So, that's fine then
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1609415770
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/impl/DefaultLogStorageFactory.java:
##########
@@ -105,9 +108,18 @@ public DefaultLogStorageFactory(String nodeName, Supplier<Path> logPathSupplier)
);
}
- /** {@inheritDoc} */
@Override
- public void start() {
+ public CompletableFuture<Void> startAsync() {
+ // This is effectively a sync implementation.
+ try {
+ start();
+ return nullCompletedFuture();
+ } catch (RuntimeException ex) {
+ return failedFuture(ex);
+ }
+ }
+
Review Comment:
This is not that simple. This method is not expected to do everything asynchronously, it's divided into a sync and an async part. Have a look at other components, for example. So I think the current approach is fine.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "tmgodinho (via GitHub)" <gi...@apache.org>.
tmgodinho commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1607873201
##########
modules/table/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceDistributedTest.java:
##########
@@ -1320,6 +1328,7 @@ void start() {
nodeCfgMgr,
failureProcessor,
clusterService,
+ logStorageFactoryComponent, // Check when to stop!!
Review Comment:
Old comment sorry.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch merged PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "tmgodinho (via GitHub)" <gi...@apache.org>.
tmgodinho commented on PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#issuecomment-2119922713
Hey @rpuch and @sashapolo, could you please have a look at these 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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "tmgodinho (via GitHub)" <gi...@apache.org>.
tmgodinho commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1607879682
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/MockitoTestUtils.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.testframework;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.withSettings;
+
+import java.util.function.Supplier;
+import org.mockito.Answers;
+import org.mockito.invocation.InvocationOnMock;
+
+/** Some utilities for tests that use Mockito. */
+public class MockitoTestUtils {
+ private MockitoTestUtils() {
+ // Intentionally left blank.
+ }
+
+ /**
+ * Creates a mockito stub only spy. Similar to {@link org.mockito.Mockito#spy(Object)} but does not track invocations.
+ * Useful to add append functionality to existing methods.
+ *
+ * @param <T> The type of the spied object.
+ * @param spiedInstanceSupplier Supplier for the spied instance. Removes the need for temporary variables.
+ * @return The created mockito spy.
+ */
+ public static <T> T spyStubOnly(Supplier<T> spiedInstanceSupplier) {
+ T instance = spiedInstanceSupplier.get();
Review Comment:
It doesn't. This was a relic from a previous iteration. Will change this according to the prev comment.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1608338973
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/LogStorageFactory.java:
##########
@@ -17,13 +17,17 @@
package org.apache.ignite.internal.raft.storage;
+import static org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
+
+import java.util.concurrent.CompletableFuture;
import org.apache.ignite.internal.close.ManuallyCloseable;
import org.apache.ignite.internal.components.LogSyncer;
+import org.apache.ignite.internal.manager.IgniteComponent;
import org.apache.ignite.raft.jraft.option.RaftOptions;
import org.apache.ignite.raft.jraft.storage.LogStorage;
/** Log storage factory interface. */
-public interface LogStorageFactory extends ManuallyCloseable, LogSyncer {
+public interface LogStorageFactory extends ManuallyCloseable, LogSyncer, IgniteComponent {
Review Comment:
Since this is now an `IgniteComponent`, we can remove the `ManuallyCloseable` inheritance
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/LogStorageFactory.java:
##########
@@ -43,4 +47,16 @@ public interface LogStorageFactory extends ManuallyCloseable, LogSyncer {
*/
@Override
void close();
+
+ @Override
+ default CompletableFuture<Void> startAsync() {
+ start();
+ return nullCompletedFuture();
+ }
+
+ @Override
+ default CompletableFuture<Void> stopAsync() {
+ close();
+ return nullCompletedFuture();
+ }
Review Comment:
I would propose to remove `start` and `close` methods completely, they are superseded by the similar methods from `IgniteComponent`
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22242 Inject LogStorageFactory dependency in Loza & JRaftServer [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #3757:
URL: https://github.com/apache/ignite-3/pull/3757#discussion_r1608244140
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/storage/LogStorageFactory.java:
##########
@@ -43,4 +47,16 @@ public interface LogStorageFactory extends ManuallyCloseable, LogSyncer {
*/
@Override
void close();
+
+ @Override
+ default CompletableFuture<Void> startAsync() {
+ start();
+ return nullCompletedFuture();
+ }
+
+ @Override
+ default CompletableFuture<Void> stopAsync() {
+ close();
+ return nullCompletedFuture();
+ }
Review Comment:
Is a good idea to provide default implementation there?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org