[GitHub] [ignite] petrov-mg commented on a diff in pull request #10482: IGNITE-18545 Fixes task execution options propagation.
petrov-mg commented on code in PR #10482: URL: https://github.com/apache/ignite/pull/10482#discussion_r1083058621 ## modules/core/src/main/java/org/apache/ignite/internal/processors/task/GridTaskProcessor.java: ## @@ -626,21 +540,10 @@ private ComputeTaskInternalFuture startTask( else taskClsName = taskCls != null ? taskCls.getName() : taskName; -// Get values from thread-local context. -Map map = thCtx.get(); - -if (map == null) -map = EMPTY_ENUM_MAP; -else -// Reset thread-local context. -thCtx.set(null); - -if (map.get(TC_SKIP_AUTH) == null) +if (!opts.isAuthenticationDisabled()) ctx.security().authorize(taskClsName, SecurityPermission.TASK_EXECUTE); -Long timeout = (Long)map.get(TC_TIMEOUT); - -long timeout0 = timeout == null || timeout == 0 ? Long.MAX_VALUE : timeout; +long timeout0 = opts.timeout().orElse(Long.MAX_VALUE); Review Comment: Fixed. I think it's better not to return Optional in TaskExecutionOptions#timeout() 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [ignite] petrov-mg commented on a diff in pull request #10482: IGNITE-18545 Fixes task execution options propagation.
petrov-mg commented on code in PR #10482:
URL: https://github.com/apache/ignite/pull/10482#discussion_r1083060832
##
modules/core/src/test/java/org/apache/ignite/internal/processors/compute/TaskOptionsPropagationTest.java:
##
@@ -0,0 +1,315 @@
+/*
+ * 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.processors.compute;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.compute.ComputeJob;
+import org.apache.ignite.compute.ComputeTask;
+import org.apache.ignite.compute.ComputeTaskAdapter;
+import org.apache.ignite.compute.ComputeTaskSession;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.util.lang.RunnableX;
+import org.apache.ignite.lang.IgniteCallable;
+import org.apache.ignite.lang.IgniteClosure;
+import org.apache.ignite.lang.IgniteReducer;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.resources.TaskSessionResource;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CALLABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CLOSURE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.RUNNABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.TASK;
+import static org.apache.ignite.testframework.GridTestUtils.runAsync;
+
+/** */
+public class TaskOptionsPropagationTest extends GridCommonAbstractTest {
+/** */
+private static final String TEST_TASK_NAME = "test-name";
+
+/** {@inheritDoc} */
+@Override protected void beforeTestsStarted() throws Exception {
+super.beforeTestsStarted();
+
+startGrid();
+
+grid().createCache(DEFAULT_CACHE_NAME).put(0, 0);
+}
+
+/** */
+@Test
+public void testUserTaskOptionsWithPrecedingSystemTaskExecution() throws
Exception {
+try (IgniteEx cli = startClientGrid(1)) {
+cli.compute().withName(TEST_TASK_NAME).affinityCall(
+DEFAULT_CACHE_NAME,
+0,
+new TestCallable(TEST_TASK_NAME)
+);
+}
+}
+
+/** */
+@Test
+public void testComputeSharedAcrossMultipleThreads() throws Exception {
+IgniteCompute compute = grid().compute();
+
+compute.withName(TEST_TASK_NAME);
+
+runAsync(() -> compute.call(new
TestCallable(TestCallable.class.getName(.get();
+
+compute.call(new TestCallable(TEST_TASK_NAME));
+}
+
+/** */
+@Test
+public void testTaskExecutionOptionsReset() throws Exception {
+check(TASK, (c, t) -> c.execute((ComputeTask)t, null));
Review Comment:
Just refactored tests a bit so dedicated enum with computation types is no
longer needed. Please take a look again.
--
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
[GitHub] [ignite] petrov-mg commented on a diff in pull request #10482: IGNITE-18545 Fixes task execution options propagation.
petrov-mg commented on code in PR #10482:
URL: https://github.com/apache/ignite/pull/10482#discussion_r1083060832
##
modules/core/src/test/java/org/apache/ignite/internal/processors/compute/TaskOptionsPropagationTest.java:
##
@@ -0,0 +1,315 @@
+/*
+ * 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.processors.compute;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.compute.ComputeJob;
+import org.apache.ignite.compute.ComputeTask;
+import org.apache.ignite.compute.ComputeTaskAdapter;
+import org.apache.ignite.compute.ComputeTaskSession;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.util.lang.RunnableX;
+import org.apache.ignite.lang.IgniteCallable;
+import org.apache.ignite.lang.IgniteClosure;
+import org.apache.ignite.lang.IgniteReducer;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.resources.TaskSessionResource;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CALLABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CLOSURE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.RUNNABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.TASK;
+import static org.apache.ignite.testframework.GridTestUtils.runAsync;
+
+/** */
+public class TaskOptionsPropagationTest extends GridCommonAbstractTest {
+/** */
+private static final String TEST_TASK_NAME = "test-name";
+
+/** {@inheritDoc} */
+@Override protected void beforeTestsStarted() throws Exception {
+super.beforeTestsStarted();
+
+startGrid();
+
+grid().createCache(DEFAULT_CACHE_NAME).put(0, 0);
+}
+
+/** */
+@Test
+public void testUserTaskOptionsWithPrecedingSystemTaskExecution() throws
Exception {
+try (IgniteEx cli = startClientGrid(1)) {
+cli.compute().withName(TEST_TASK_NAME).affinityCall(
+DEFAULT_CACHE_NAME,
+0,
+new TestCallable(TEST_TASK_NAME)
+);
+}
+}
+
+/** */
+@Test
+public void testComputeSharedAcrossMultipleThreads() throws Exception {
+IgniteCompute compute = grid().compute();
+
+compute.withName(TEST_TASK_NAME);
+
+runAsync(() -> compute.call(new
TestCallable(TestCallable.class.getName(.get();
+
+compute.call(new TestCallable(TEST_TASK_NAME));
+}
+
+/** */
+@Test
+public void testTaskExecutionOptionsReset() throws Exception {
+check(TASK, (c, t) -> c.execute((ComputeTask)t, null));
Review Comment:
Just refactored tests a bit so dedicated enum with computation classes is no
longer needed. Please take a look again
--
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
[GitHub] [ignite] petrov-mg commented on a diff in pull request #10482: IGNITE-18545 Fixes task execution options propagation.
petrov-mg commented on code in PR #10482: URL: https://github.com/apache/ignite/pull/10482#discussion_r1083058621 ## modules/core/src/main/java/org/apache/ignite/internal/processors/task/GridTaskProcessor.java: ## @@ -626,21 +540,10 @@ private ComputeTaskInternalFuture startTask( else taskClsName = taskCls != null ? taskCls.getName() : taskName; -// Get values from thread-local context. -Map map = thCtx.get(); - -if (map == null) -map = EMPTY_ENUM_MAP; -else -// Reset thread-local context. -thCtx.set(null); - -if (map.get(TC_SKIP_AUTH) == null) +if (!opts.isAuthenticationDisabled()) ctx.security().authorize(taskClsName, SecurityPermission.TASK_EXECUTE); -Long timeout = (Long)map.get(TC_TIMEOUT); - -long timeout0 = timeout == null || timeout == 0 ? Long.MAX_VALUE : timeout; +long timeout0 = opts.timeout().orElse(Long.MAX_VALUE); Review Comment: Fixed. I think it's better not to return optional in TaskExecutionOptions#timeout() 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-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [ignite] petrov-mg commented on a diff in pull request #10482: IGNITE-18545 Fixes task execution options propagation.
petrov-mg commented on code in PR #10482:
URL: https://github.com/apache/ignite/pull/10482#discussion_r1083056001
##
modules/core/src/test/java/org/apache/ignite/internal/processors/compute/TaskOptionsPropagationTest.java:
##
@@ -0,0 +1,315 @@
+/*
+ * 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.processors.compute;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.compute.ComputeJob;
+import org.apache.ignite.compute.ComputeTask;
+import org.apache.ignite.compute.ComputeTaskAdapter;
+import org.apache.ignite.compute.ComputeTaskSession;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.util.lang.RunnableX;
+import org.apache.ignite.lang.IgniteCallable;
+import org.apache.ignite.lang.IgniteClosure;
+import org.apache.ignite.lang.IgniteReducer;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.resources.TaskSessionResource;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CALLABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CLOSURE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.RUNNABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.TASK;
+import static org.apache.ignite.testframework.GridTestUtils.runAsync;
+
+/** */
+public class TaskOptionsPropagationTest extends GridCommonAbstractTest {
+/** */
+private static final String TEST_TASK_NAME = "test-name";
+
+/** {@inheritDoc} */
+@Override protected void beforeTestsStarted() throws Exception {
+super.beforeTestsStarted();
+
+startGrid();
+
+grid().createCache(DEFAULT_CACHE_NAME).put(0, 0);
+}
+
+/** */
+@Test
+public void testUserTaskOptionsWithPrecedingSystemTaskExecution() throws
Exception {
+try (IgniteEx cli = startClientGrid(1)) {
+cli.compute().withName(TEST_TASK_NAME).affinityCall(
+DEFAULT_CACHE_NAME,
+0,
+new TestCallable(TEST_TASK_NAME)
+);
+}
+}
+
+/** */
+@Test
+public void testComputeSharedAcrossMultipleThreads() throws Exception {
+IgniteCompute compute = grid().compute();
+
+compute.withName(TEST_TASK_NAME);
+
+runAsync(() -> compute.call(new
TestCallable(TestCallable.class.getName(.get();
+
+compute.call(new TestCallable(TEST_TASK_NAME));
+}
+
+/** */
+@Test
+public void testTaskExecutionOptionsReset() throws Exception {
+check(TASK, (c, t) -> c.execute((ComputeTask)t, null));
+check(TASK, (c, t) -> c.executeAsync((ComputeTask)t, null).get());
+
+check(CALLABLE, (c, t) -> c.call((IgniteCallable)t));
+check(CALLABLE, (c, t) -> c.callAsync((IgniteCallable)t).get());
+
+check(CALLABLE, (c, t) -> c.call((toList((IgniteCallable)t;
+check(CALLABLE, (c, t) ->
c.callAsync(toList((IgniteCallable)t)).get());
+
+check(CALLABLE, (c, t) -> c.call(toList((IgniteCallable)t), new
TestReducer()));
+check(CALLABLE, (c, t) -> c.callAsync(toList((IgniteCallable)t),
new TestReducer()).get());
+
+check(RUNNABLE, (c, t) -> c.run((IgniteRunnable)t));
+check(RUNNABLE, (c, t) -> c.runAsync((IgniteRunnable)t).get());
+
+check(RUNNABLE, (c, t) -> c.run(toList((IgniteRunnable)t)));
+check(RUNNABLE, (c, t) -> c.runAsync(toList((IgniteRunnable)t)).get());
+
+check(RUNNABLE, (c, t) -> c.broadcast((IgniteRunnable)t));
+check(RUNNABL
[GitHub] [ignite-3] lowka commented on a diff in pull request #1528: IGNITE-18422: Sql. Match number of dynamic parameters with given parameters
lowka commented on code in PR #1528:
URL: https://github.com/apache/ignite-3/pull/1528#discussion_r1082958726
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java:
##
@@ -363,27 +365,54 @@ public QueryChecker planEquals(String plan) {
return this;
}
+/**
+ * Specifies whether to execute {@code EXPLAIN PLAN FOR} statement before
running a query or not.
+ * The default value is {@code true}.
+ *
+ * If the value is set to {@code false} but either plan or at least one
plan matcher is specified {@link #check()}
+ * throws {@link IllegalStateException}.
+ *
+ *
+ * @param value whether to execute EXPLAIN PLAN statement or not.
+ * @return this
Review Comment:
Fixed.
--
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
[GitHub] [ignite-3] lowka commented on a diff in pull request #1528: IGNITE-18422: Sql. Match number of dynamic parameters with given parameters
lowka commented on code in PR #1528:
URL: https://github.com/apache/ignite-3/pull/1528#discussion_r1082957978
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java:
##
@@ -459,15 +459,23 @@ private boolean isSystemFieldName(String alias) {
/** {@inheritDoc} */
@Override
protected void inferUnknownTypes(RelDataType inferredType,
SqlValidatorScope scope, SqlNode node) {
-if (node instanceof SqlDynamicParam &&
inferredType.equals(unknownType)) {
-Object param = parameters[((SqlDynamicParam) node).getIndex()];
-RelDataType type;
-if (param == null) {
-type = typeFactory().createSqlType(SqlTypeName.NULL);
+if (node instanceof SqlDynamicParam) {
+int index = ((SqlDynamicParam) node).getIndex();
+if (index >= parameters.length) {
+throw newValidationError(node, RESOURCE.dynamicParamIllegal());
Review Comment:
fixed
--
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
[GitHub] [ignite] NSAmelchev commented on a diff in pull request #10482: IGNITE-18545 Fixes task execution options propagation.
NSAmelchev commented on code in PR #10482: URL: https://github.com/apache/ignite/pull/10482#discussion_r1082947629 ## modules/core/src/main/java/org/apache/ignite/internal/processors/task/GridTaskProcessor.java: ## @@ -626,21 +540,10 @@ private ComputeTaskInternalFuture startTask( else taskClsName = taskCls != null ? taskCls.getName() : taskName; -// Get values from thread-local context. -Map map = thCtx.get(); - -if (map == null) -map = EMPTY_ENUM_MAP; -else -// Reset thread-local context. -thCtx.set(null); - -if (map.get(TC_SKIP_AUTH) == null) +if (!opts.isAuthenticationDisabled()) ctx.security().authorize(taskClsName, SecurityPermission.TASK_EXECUTE); -Long timeout = (Long)map.get(TC_TIMEOUT); - -long timeout0 = timeout == null || timeout == 0 ? Long.MAX_VALUE : timeout; +long timeout0 = opts.timeout().orElse(Long.MAX_VALUE); Review Comment: Timeout should be `Long.MAX_VALUE` in case `timeout == 0` too -- 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
[GitHub] [ignite-3] sashapolo opened a new pull request, #1561: IGNITE-18600 Save Meta Storage watch data to the Vault
sashapolo opened a new pull request, #1561: URL: https://github.com/apache/ignite-3/pull/1561 https://issues.apache.org/jira/browse/IGNITE-18600 -- 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
[GitHub] [ignite] NSAmelchev commented on a diff in pull request #10482: IGNITE-18545 Fixes task execution options propagation.
NSAmelchev commented on code in PR #10482:
URL: https://github.com/apache/ignite/pull/10482#discussion_r1082925214
##
modules/core/src/test/java/org/apache/ignite/internal/processors/compute/TaskOptionsPropagationTest.java:
##
@@ -0,0 +1,315 @@
+/*
+ * 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.processors.compute;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.compute.ComputeJob;
+import org.apache.ignite.compute.ComputeTask;
+import org.apache.ignite.compute.ComputeTaskAdapter;
+import org.apache.ignite.compute.ComputeTaskSession;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.util.lang.RunnableX;
+import org.apache.ignite.lang.IgniteCallable;
+import org.apache.ignite.lang.IgniteClosure;
+import org.apache.ignite.lang.IgniteReducer;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.resources.TaskSessionResource;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CALLABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CLOSURE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.RUNNABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.TASK;
+import static org.apache.ignite.testframework.GridTestUtils.runAsync;
+
+/** */
+public class TaskOptionsPropagationTest extends GridCommonAbstractTest {
+/** */
+private static final String TEST_TASK_NAME = "test-name";
+
+/** {@inheritDoc} */
+@Override protected void beforeTestsStarted() throws Exception {
+super.beforeTestsStarted();
+
+startGrid();
+
+grid().createCache(DEFAULT_CACHE_NAME).put(0, 0);
+}
+
+/** */
+@Test
+public void testUserTaskOptionsWithPrecedingSystemTaskExecution() throws
Exception {
+try (IgniteEx cli = startClientGrid(1)) {
+cli.compute().withName(TEST_TASK_NAME).affinityCall(
+DEFAULT_CACHE_NAME,
+0,
+new TestCallable(TEST_TASK_NAME)
+);
+}
+}
+
+/** */
+@Test
+public void testComputeSharedAcrossMultipleThreads() throws Exception {
+IgniteCompute compute = grid().compute();
+
+compute.withName(TEST_TASK_NAME);
+
+runAsync(() -> compute.call(new
TestCallable(TestCallable.class.getName(.get();
+
+compute.call(new TestCallable(TEST_TASK_NAME));
+}
+
+/** */
+@Test
+public void testTaskExecutionOptionsReset() throws Exception {
+check(TASK, (c, t) -> c.execute((ComputeTask)t, null));
+check(TASK, (c, t) -> c.executeAsync((ComputeTask)t, null).get());
+
+check(CALLABLE, (c, t) -> c.call((IgniteCallable)t));
+check(CALLABLE, (c, t) -> c.callAsync((IgniteCallable)t).get());
+
+check(CALLABLE, (c, t) -> c.call((toList((IgniteCallable)t;
+check(CALLABLE, (c, t) ->
c.callAsync(toList((IgniteCallable)t)).get());
+
+check(CALLABLE, (c, t) -> c.call(toList((IgniteCallable)t), new
TestReducer()));
+check(CALLABLE, (c, t) -> c.callAsync(toList((IgniteCallable)t),
new TestReducer()).get());
+
+check(RUNNABLE, (c, t) -> c.run((IgniteRunnable)t));
+check(RUNNABLE, (c, t) -> c.runAsync((IgniteRunnable)t).get());
+
+check(RUNNABLE, (c, t) -> c.run(toList((IgniteRunnable)t)));
+check(RUNNABLE, (c, t) -> c.runAsync(toList((IgniteRunnable)t)).get());
+
+check(RUNNABLE, (c, t) -> c.broadcast((IgniteRunnable)t));
+check(RUNNAB
[GitHub] [ignite] timoninmaxim merged pull request #10314: IGNITE-17029 Incremental snapshot creation implemented
timoninmaxim merged PR #10314: URL: https://github.com/apache/ignite/pull/10314 -- 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
[GitHub] [ignite] NSAmelchev commented on a diff in pull request #10482: IGNITE-18545 Fixes task execution options propagation.
NSAmelchev commented on code in PR #10482:
URL: https://github.com/apache/ignite/pull/10482#discussion_r1082924047
##
modules/core/src/test/java/org/apache/ignite/internal/processors/compute/TaskOptionsPropagationTest.java:
##
@@ -0,0 +1,315 @@
+/*
+ * 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.processors.compute;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.compute.ComputeJob;
+import org.apache.ignite.compute.ComputeTask;
+import org.apache.ignite.compute.ComputeTaskAdapter;
+import org.apache.ignite.compute.ComputeTaskSession;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.util.lang.RunnableX;
+import org.apache.ignite.lang.IgniteCallable;
+import org.apache.ignite.lang.IgniteClosure;
+import org.apache.ignite.lang.IgniteReducer;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.resources.TaskSessionResource;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CALLABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CLOSURE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.RUNNABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.TASK;
+import static org.apache.ignite.testframework.GridTestUtils.runAsync;
+
+/** */
+public class TaskOptionsPropagationTest extends GridCommonAbstractTest {
+/** */
+private static final String TEST_TASK_NAME = "test-name";
+
+/** {@inheritDoc} */
+@Override protected void beforeTestsStarted() throws Exception {
+super.beforeTestsStarted();
+
+startGrid();
+
+grid().createCache(DEFAULT_CACHE_NAME).put(0, 0);
+}
+
+/** */
+@Test
+public void testUserTaskOptionsWithPrecedingSystemTaskExecution() throws
Exception {
+try (IgniteEx cli = startClientGrid(1)) {
+cli.compute().withName(TEST_TASK_NAME).affinityCall(
+DEFAULT_CACHE_NAME,
+0,
+new TestCallable(TEST_TASK_NAME)
+);
+}
+}
+
+/** */
+@Test
+public void testComputeSharedAcrossMultipleThreads() throws Exception {
+IgniteCompute compute = grid().compute();
+
+compute.withName(TEST_TASK_NAME);
+
+runAsync(() -> compute.call(new
TestCallable(TestCallable.class.getName(.get();
+
+compute.call(new TestCallable(TEST_TASK_NAME));
+}
+
+/** */
+@Test
+public void testTaskExecutionOptionsReset() throws Exception {
+check(TASK, (c, t) -> c.execute((ComputeTask)t, null));
Review Comment:
Let's cast to `ComputeTask` as in the code below.
--
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
[GitHub] [ignite] NSAmelchev commented on a diff in pull request #10482: IGNITE-18545 Fixes task execution options propagation.
NSAmelchev commented on code in PR #10482:
URL: https://github.com/apache/ignite/pull/10482#discussion_r1082922705
##
modules/core/src/test/java/org/apache/ignite/internal/processors/compute/TaskOptionsPropagationTest.java:
##
@@ -0,0 +1,315 @@
+/*
+ * 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.processors.compute;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.compute.ComputeJob;
+import org.apache.ignite.compute.ComputeTask;
+import org.apache.ignite.compute.ComputeTaskAdapter;
+import org.apache.ignite.compute.ComputeTaskSession;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.util.lang.RunnableX;
+import org.apache.ignite.lang.IgniteCallable;
+import org.apache.ignite.lang.IgniteClosure;
+import org.apache.ignite.lang.IgniteReducer;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.resources.TaskSessionResource;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CALLABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.CLOSURE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.RUNNABLE;
+import static
org.apache.ignite.internal.processors.compute.TaskOptionsPropagationTest.ComputationType.TASK;
+import static org.apache.ignite.testframework.GridTestUtils.runAsync;
+
+/** */
+public class TaskOptionsPropagationTest extends GridCommonAbstractTest {
+/** */
+private static final String TEST_TASK_NAME = "test-name";
+
+/** {@inheritDoc} */
+@Override protected void beforeTestsStarted() throws Exception {
+super.beforeTestsStarted();
+
+startGrid();
+
+grid().createCache(DEFAULT_CACHE_NAME).put(0, 0);
+}
+
+/** */
+@Test
+public void testUserTaskOptionsWithPrecedingSystemTaskExecution() throws
Exception {
+try (IgniteEx cli = startClientGrid(1)) {
+cli.compute().withName(TEST_TASK_NAME).affinityCall(
+DEFAULT_CACHE_NAME,
+0,
+new TestCallable(TEST_TASK_NAME)
+);
+}
+}
+
+/** */
+@Test
+public void testComputeSharedAcrossMultipleThreads() throws Exception {
+IgniteCompute compute = grid().compute();
+
+compute.withName(TEST_TASK_NAME);
+
+runAsync(() -> compute.call(new
TestCallable(TestCallable.class.getName(.get();
+
+compute.call(new TestCallable(TEST_TASK_NAME));
+}
+
+/** */
+@Test
+public void testTaskExecutionOptionsReset() throws Exception {
+check(TASK, (c, t) -> c.execute((ComputeTask)t, null));
+check(TASK, (c, t) -> c.executeAsync((ComputeTask)t, null).get());
+
+check(CALLABLE, (c, t) -> c.call((IgniteCallable)t));
+check(CALLABLE, (c, t) -> c.callAsync((IgniteCallable)t).get());
+
+check(CALLABLE, (c, t) -> c.call((toList((IgniteCallable)t;
+check(CALLABLE, (c, t) ->
c.callAsync(toList((IgniteCallable)t)).get());
+
+check(CALLABLE, (c, t) -> c.call(toList((IgniteCallable)t), new
TestReducer()));
+check(CALLABLE, (c, t) -> c.callAsync(toList((IgniteCallable)t),
new TestReducer()).get());
+
+check(RUNNABLE, (c, t) -> c.run((IgniteRunnable)t));
+check(RUNNABLE, (c, t) -> c.runAsync((IgniteRunnable)t).get());
+
+check(RUNNABLE, (c, t) -> c.run(toList((IgniteRunnable)t)));
+check(RUNNABLE, (c, t) -> c.runAsync(toList((IgniteRunnable)t)).get());
+
+check(RUNNABLE, (c, t) -> c.broadcast((IgniteRunnable)t));
+check(RUNNAB
[GitHub] [ignite-3] lowka commented on a diff in pull request #1528: IGNITE-18422: Sql. Match number of dynamic parameters with given parameters
lowka commented on code in PR #1528:
URL: https://github.com/apache/ignite-3/pull/1528#discussion_r1082915985
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java:
##
@@ -114,6 +119,35 @@ public IgniteSqlValidator(SqlOperatorTable opTab,
CalciteCatalogReader catalogRe
this.parameters = parameters;
}
+/** {@inheritDoc} */
+@Override
+public SqlNode validate(SqlNode topNode) {
+this.dynamicParameterCount = 0;
+try {
+SqlNode topNodeToValidate;
+// Calcite fails to validate a query when its top node is EXPLAIN
PLAN FOR
+// java.lang.NullPointerException: namespace for
+// at
org.apache.calcite.sql.validate.SqlValidatorImpl.getNamespaceOrThrow(SqlValidatorImpl.java:1280)
+if (topNode instanceof SqlExplain) {
+topNodeToValidate = ((SqlExplain) topNode).getExplicandum();
+// We do not validate dynamic parameters in case of EXPLAIN
FOR queries since they may be omitted.
+// TODO: We must enable this check for EXPLAIN ANALYSE flavour
for EXPLAIN FOR queries.
Review Comment:
We should open an issue for this one.
--
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
[GitHub] [ignite] NSAmelchev commented on a diff in pull request #10393: IGNITE-18236 Cache objects transformation
NSAmelchev commented on code in PR #10393:
URL: https://github.com/apache/ignite/pull/10393#discussion_r1082732858
##
modules/core/src/main/java/org/apache/ignite/events/EventType.java:
##
@@ -526,6 +526,17 @@ public interface EventType {
*/
public static final int EVT_CACHE_OBJECT_UNLOCKED = 67;
+/**
+ * Built-in event type: cache object was transformed.
+ *
+ * NOTE: all types in range from 1 to 1000 are reserved for
+ * internal Ignite events and should not be used by user-defined events.
+ *
+ * @see CacheEvent
+ */
+
Review Comment:
Unnecessary line break
--
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
[GitHub] [ignite] NSAmelchev commented on a diff in pull request #10393: IGNITE-18236 Cache objects transformation
NSAmelchev commented on code in PR #10393:
URL: https://github.com/apache/ignite/pull/10393#discussion_r1082732023
##
modules/core/src/main/java/org/apache/ignite/events/CacheObjectTransformedEvent.java:
##
@@ -0,0 +1,82 @@
+/*
+ * 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.events;
+
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.lang.IgniteExperimental;
+
+/**
+ *
+ */
+@IgniteExperimental
+public class CacheObjectTransformedEvent extends EventAdapter {
+/** */
+private static final long serialVersionUID = 0L;
+
+/** Original cache object bytes. */
+private final byte[] original;
+
+/** Transformed cache object bytes. */
+private final byte[] transformed;
+
+/** Restore operation. */
+private final boolean restore;
+
+/**
+ * @param nodeNode.
+ * @param msg Message.
+ * @param typeType.
+ * @param originalOriginal cache object bytes.
+ * @param transformed Transformed cache object bytes.
+ * @param restore Restore operation flag.
+ */
+public CacheObjectTransformedEvent(
+ClusterNode node,
+String msg,
+int type,
+byte[] original,
Review Comment:
Are these bytes actual without offset?
--
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
[GitHub] [ignite] NSAmelchev commented on a diff in pull request #10393: IGNITE-18236 Cache objects transformation
NSAmelchev commented on code in PR #10393:
URL: https://github.com/apache/ignite/pull/10393#discussion_r1082728668
##
modules/core/src/main/java/org/apache/ignite/configuration/IgniteConfiguration.java:
##
@@ -416,6 +417,9 @@ public class IgniteConfiguration {
/** Failover SPI. */
private FailoverSpi[] failSpi;
+/** Cache object transform spi. */
Review Comment:
not an SPI anymore
--
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
[GitHub] [ignite] NSAmelchev commented on pull request #10393: IGNITE-18236 Cache objects transformation
NSAmelchev commented on PR #10393: URL: https://github.com/apache/ignite/pull/10393#issuecomment-1398588256 Should we check that cluster nodes have the same `CacheObjectTransformer`? -- 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
[GitHub] [ignite-3] ibessonov merged pull request #1560: IGNITE-18598 Fix compilation after merge
ibessonov merged PR #1560: URL: https://github.com/apache/ignite-3/pull/1560 -- 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
[GitHub] [ignite-3] sashapolo opened a new pull request, #1560: IGNITE-18598 Fix compilation after merge
sashapolo opened a new pull request, #1560: URL: https://github.com/apache/ignite-3/pull/1560 https://issues.apache.org/jira/browse/IGNITE-18598 -- 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
[GitHub] [ignite-3] ibessonov merged pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
ibessonov merged PR #1490: URL: https://github.com/apache/ignite-3/pull/1490 -- 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
[GitHub] [ignite-3] rpuch opened a new pull request, #1559: IGNITE-18531 Destroy indices when destroying VolatilePageMemoryMvPartitionStorage
rpuch opened a new pull request, #1559: URL: https://github.com/apache/ignite-3/pull/1559 https://issues.apache.org/jira/browse/IGNITE-18531 -- 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
[GitHub] [ignite-3] SammyVimes merged pull request #1558: IGNITE-18597 Strip tail zeroes in RocksUtils#incrementArray
SammyVimes merged PR #1558: URL: https://github.com/apache/ignite-3/pull/1558 -- 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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082595722
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java:
##
@@ -842,11 +753,9 @@ public boolean hasNext() {
}
try {
-try {
-return innerIterFut.thenApply(Iterator::hasNext).get();
-} catch (InterruptedException | ExecutionException e) {
-throw new MetaStorageException(CURSOR_EXECUTION_ERR, e);
-}
+return innerCursorFut.thenApply(Iterator::hasNext).get();
+} catch (InterruptedException | ExecutionException e) {
+throw new MetaStorageException(CURSOR_EXECUTION_ERR, e);
Review Comment:
Got it, will fix
--
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
[GitHub] [ignite-3] zstan commented on a diff in pull request #1550: IGNITE-18585 Sql. Introduce cache for serialized RelNodes representation
zstan commented on code in PR #1550:
URL: https://github.com/apache/ignite-3/pull/1550#discussion_r1082590034
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##
@@ -83,6 +86,13 @@
* ExecutionServiceImpl. TODO Documentation
https://issues.apache.org/jira/browse/IGNITE-15859
*/
public class ExecutionServiceImpl implements ExecutionService,
TopologyEventHandler {
+private static final int CACHE_SIZE = 1024;
+
+private static final ConcurrentMap PHYS_NODES_REPR =
Caffeine.newBuilder()
Review Comment:
ok, maybe you right .. .i remove static from 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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] ibessonov commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
ibessonov commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082586412
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageLearnerListener.java:
##
@@ -0,0 +1,73 @@
+/*
+ * 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.metastorage.server.raft;
+
+import java.nio.file.Path;
+import java.util.Iterator;
+import java.util.function.Consumer;
+import org.apache.ignite.internal.metastorage.server.KeyValueStorage;
+import org.apache.ignite.internal.raft.ReadCommand;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.CommandClosure;
+import org.apache.ignite.internal.raft.service.RaftGroupListener;
+
+/**
+ * Meta Storage Raft group listener for learner nodes. These nodes ignore read
and cursor-related commands.
+ */
+public class MetaStorageLearnerListener implements RaftGroupListener {
+private final KeyValueStorage storage;
+
+private final MetaStorageWriteHandler writeHandler;
+
+public MetaStorageLearnerListener(KeyValueStorage storage) {
+this.storage = storage;
+this.writeHandler = new MetaStorageWriteHandler(storage);
+}
+
+@Override
+public void onRead(Iterator> iterator) {
+throw new UnsupportedOperationException("Reads should not be sent to
learners");
+}
+
+@Override
+public void onWrite(Iterator> iter) {
+while (iter.hasNext()) {
+CommandClosure clo = iter.next();
+
+if (!writeHandler.handleWriteCommand(clo)) {
+// Ignore all commands that are not handled by the
writeHandler.
+clo.result(null);
Review Comment:
Ok, my bad, I wasn't careful enough while reading this 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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] ibessonov commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
ibessonov commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082584043
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java:
##
@@ -842,11 +753,9 @@ public boolean hasNext() {
}
try {
-try {
-return innerIterFut.thenApply(Iterator::hasNext).get();
-} catch (InterruptedException | ExecutionException e) {
-throw new MetaStorageException(CURSOR_EXECUTION_ERR, e);
-}
+return innerCursorFut.thenApply(Iterator::hasNext).get();
+} catch (InterruptedException | ExecutionException e) {
+throw new MetaStorageException(CURSOR_EXECUTION_ERR, e);
Review Comment:
Other similar catches in this class, that handle interrupted exceptions
--
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
[GitHub] [ignite-3] ibessonov commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
ibessonov commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082584043
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java:
##
@@ -842,11 +753,9 @@ public boolean hasNext() {
}
try {
-try {
-return innerIterFut.thenApply(Iterator::hasNext).get();
-} catch (InterruptedException | ExecutionException e) {
-throw new MetaStorageException(CURSOR_EXECUTION_ERR, e);
-}
+return innerCursorFut.thenApply(Iterator::hasNext).get();
+} catch (InterruptedException | ExecutionException e) {
+throw new MetaStorageException(CURSOR_EXECUTION_ERR, e);
Review Comment:
Other similar catches in 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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] ygerzhedovich commented on a diff in pull request #1469: IGNITE-18227: refactoring scan nodes and add support RO index scans.
ygerzhedovich commented on code in PR #1469:
URL: https://github.com/apache/ignite-3/pull/1469#discussion_r1082583163
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlAsynchronousApiTest.java:
##
@@ -343,37 +346,58 @@ public void checkTransactionsWithDml() throws Exception {
assertEquals(txManagerInternal.finished(), states.size());
}
-/** Check correctness of rw and ro transactions. */
+/** Check correctness of rw and ro transactions for table scan. */
@Test
-public void checkMixedTransactions() throws Exception {
+public void checkMixedTransactionsForTable() throws Exception {
+sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+Matcher planMatcher = containsTableScan("PUBLIC", "TEST");
+
+checkMixedTransactions(planMatcher);
+}
+
+
+/** Check correctness of rw and ro transactions for index scan. */
+@Test
+public void checkMixedTransactionsForIndex() throws Exception {
+sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+sql("CREATE INDEX TEST_IDX ON TEST(VAL0)");
+
+Matcher planMatcher = containsIndexScan("PUBLIC", "TEST",
"TEST_IDX");
+
+checkMixedTransactions(planMatcher);
+}
+
+private void checkMixedTransactions(Matcher planMatcher) throws
Exception {
IgniteSql sql = igniteSql();
if (sql instanceof ClientSql) {
return;
}
-sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
-
Session ses = sql.createSession();
for (int i = 0; i < ROW_COUNT; ++i) {
sql("INSERT INTO TEST VALUES (?, ?)", i, i);
}
-checkTx(ses, true, false, true);
-checkTx(ses, true, false, false);
-checkTx(ses, true, true, true);
-checkTx(ses, true, true, false);
-checkTx(ses, false, true, true);
-checkTx(ses, false, true, false);
-checkTx(ses, false, false, true);
-checkTx(ses, false, false, false);
+for (int roTx = 0; roTx < 2; roTx++) {
Review Comment:
fixed
--
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
[GitHub] [ignite-3] ibessonov commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
ibessonov commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082582875
##
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##
@@ -741,39 +742,27 @@ private void initDataNodesFromVaultManager() {
}
try {
-// TODO: Remove this call as part of
https://issues.apache.org/jira/browse/IGNITE-18397
-vaultMgr.get(MetaStorageManagerImpl.APPLIED_REV)
-.thenApply(appliedRevision -> appliedRevision == null ? 0L
: bytesToLong(appliedRevision.value()))
-.thenAccept(vaultAppliedRevision -> {
+long appliedRevision = metaStorageManager.appliedRevision();
+
+vaultMgr.get(zonesLogicalTopologyKey())
Review Comment:
Just a conversation
--
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
[GitHub] [ignite-3] sanpwc merged pull request #1503: IGNITE-18324 Calling tx.rollback() or tx.commit() is not failed for committed transaction.
sanpwc merged PR #1503: URL: https://github.com/apache/ignite-3/pull/1503 -- 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
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1557: IGNITE-18596 Add RaftGroupConfiguration to MvTableStorage#finishRebalancePartition
tkalkirill commented on code in PR #1557:
URL: https://github.com/apache/ignite-3/pull/1557#discussion_r1082521831
##
modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/AbstractMvTableStorageTest.java:
##
@@ -917,4 +936,26 @@ private static List
toListOfByteArrays(Cursor cursor) {
return
cursor.stream().map(ReadResult::binaryRow).map(BinaryRow::bytes).collect(toList());
}
}
+
+private static RaftGroupConfiguration createRandomRaftGroupConfiguration()
{
+Random random = new Random();
Review Comment:
Fix 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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1469: IGNITE-18227: refactoring scan nodes and add support RO index scans.
AMashenkov commented on code in PR #1469:
URL: https://github.com/apache/ignite-3/pull/1469#discussion_r1082505775
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlAsynchronousApiTest.java:
##
@@ -343,37 +346,58 @@ public void checkTransactionsWithDml() throws Exception {
assertEquals(txManagerInternal.finished(), states.size());
}
-/** Check correctness of rw and ro transactions. */
+/** Check correctness of rw and ro transactions for table scan. */
@Test
-public void checkMixedTransactions() throws Exception {
+public void checkMixedTransactionsForTable() throws Exception {
+sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+
+Matcher planMatcher = containsTableScan("PUBLIC", "TEST");
+
+checkMixedTransactions(planMatcher);
+}
+
+
+/** Check correctness of rw and ro transactions for index scan. */
+@Test
+public void checkMixedTransactionsForIndex() throws Exception {
+sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
+sql("CREATE INDEX TEST_IDX ON TEST(VAL0)");
+
+Matcher planMatcher = containsIndexScan("PUBLIC", "TEST",
"TEST_IDX");
+
+checkMixedTransactions(planMatcher);
+}
+
+private void checkMixedTransactions(Matcher planMatcher) throws
Exception {
IgniteSql sql = igniteSql();
if (sql instanceof ClientSql) {
return;
}
-sql("CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)");
-
Session ses = sql.createSession();
for (int i = 0; i < ROW_COUNT; ++i) {
sql("INSERT INTO TEST VALUES (?, ?)", i, i);
}
-checkTx(ses, true, false, true);
-checkTx(ses, true, false, false);
-checkTx(ses, true, true, true);
-checkTx(ses, true, true, false);
-checkTx(ses, false, true, true);
-checkTx(ses, false, true, false);
-checkTx(ses, false, false, true);
-checkTx(ses, false, false, false);
+for (int roTx = 0; roTx < 2; roTx++) {
Review Comment:
```suggestion
for (boolean roTx : List.of(Boolean.TRUE, Boolean.FALSE)) {
```
--
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
[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #1557: IGNITE-18596 Add RaftGroupConfiguration to MvTableStorage#finishRebalancePartition
SammyVimes commented on code in PR #1557:
URL: https://github.com/apache/ignite-3/pull/1557#discussion_r1082490021
##
modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/AbstractMvTableStorageTest.java:
##
@@ -917,4 +936,26 @@ private static List
toListOfByteArrays(Cursor cursor) {
return
cursor.stream().map(ReadResult::binaryRow).map(BinaryRow::bytes).collect(toList());
}
}
+
+private static RaftGroupConfiguration createRandomRaftGroupConfiguration()
{
+Random random = new Random();
Review Comment:
Better add a seed here, make tests repeatable
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1536: IGNITE-18207 Sql. Pushdown DISTINCT aggregate with no particular function
korlov42 commented on code in PR #1536:
URL: https://github.com/apache/ignite-3/pull/1536#discussion_r1082496024
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/AbstractBasicIntegrationTest.java:
##
@@ -289,6 +299,26 @@ enum JoinType {
}
}
+enum AggregateType {
+SORT(
+"ColocatedHashAggregateConverterRule",
+"MapReduceHashAggregateConverterRule",
+"ColocatedSortAggregateConverterRule"
Review Comment:
ColocatedAggregateRule is misplaced in both SORT and HASH aggregate types
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1536: IGNITE-18207 Sql. Pushdown DISTINCT aggregate with no particular function
korlov42 commented on code in PR #1536:
URL: https://github.com/apache/ignite-3/pull/1536#discussion_r1082494820
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/AbstractBasicIntegrationTest.java:
##
@@ -244,9 +241,22 @@ protected QueryProcessor getEngine() {
* @param rules Additional rules need to be disabled.
*/
static QueryChecker assertQuery(String qry, JoinType joinType, String...
rules) {
-return
AbstractBasicIntegrationTest.assertQuery(qry.replaceAll("(?i)^select", "select "
-+ Stream.concat(Arrays.stream(joinType.disabledRules),
Arrays.stream(rules))
-.collect(Collectors.joining("','", "/*+ DISABLE_RULE('", "')
*/";
+return assertQuery(qry)
+.disableRules(joinType.disabledRules)
+.disableRules(rules);
+}
+
+/**
+ * Used for join checks, disables other join rules for executing exact
join algo.
Review Comment:
I believe it's used for aggregation check, not join
--
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
[GitHub] [ignite-3] sashapolo opened a new pull request, #1558: IGNITE-18597 Strip tail zeroes in RocksUtils#incrementArray
sashapolo opened a new pull request, #1558: URL: https://github.com/apache/ignite-3/pull/1558 https://issues.apache.org/jira/browse/IGNITE-18597 -- 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
[GitHub] [ignite-3] korlov42 merged pull request #1513: IGNITE-18163: Old-style join on different column types fails with ClassCastException
korlov42 merged PR #1513: URL: https://github.com/apache/ignite-3/pull/1513 -- 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
[GitHub] [ignite-3] tkalkirill opened a new pull request, #1557: IGNITE-18596 Add RaftGroupConfiguration to MvTableStorage#finishRebalancePartition
tkalkirill opened a new pull request, #1557: URL: https://github.com/apache/ignite-3/pull/1557 https://issues.apache.org/jira/browse/IGNITE-18596 -- 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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1082427170
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/SortAggregateNode.java:
##
@@ -159,6 +161,10 @@ public void end() throws Exception {
waiting = -1;
+if (grp == null && accFactory != null) {
Review Comment:
this will produce empty row when none is expected in query like `SELECT
min(c) FROM t GROUP BY a` when table is empty. BTW, we need execution test for
this case
to initialise group properly, please, take a look at
`org.apache.ignite.internal.sql.engine.exec.rel.HashAggregateNode.Grouping#init`
--
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
[GitHub] [ignite-3] sergeyuttsel commented on a diff in pull request #1503: IGNITE-18324 Calling tx.rollback() or tx.commit() is not failed for committed transaction.
sergeyuttsel commented on code in PR #1503:
URL: https://github.com/apache/ignite-3/pull/1503#discussion_r1082417133
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadWriteTransactionImpl.java:
##
@@ -126,10 +135,14 @@ protected CompletableFuture finish(boolean commit) {
id()
);
} else {
-return CompletableFuture.completedFuture(null);
+return completedFuture(null);
}
}
);
+
+mainFinishFut.handle((res, e) -> finishFut.get().complete(null));
Review Comment:
It is need to complete future both on success and exception.
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/PartitionListener.java:
##
@@ -279,12 +280,13 @@ private void handleUpdateAllCommand(UpdateAllCommand cmd,
long commandIndex, lon
* @param cmd Command.
* @param commandIndex Index of the RAFT command.
* @param commandTerm Term of the RAFT command.
+ * @return {@code true} if transaction state was changed.
* @throws IgniteInternalException if an exception occurred during a
transaction state change.
*/
-private void handleFinishTxCommand(FinishTxCommand cmd, long commandIndex,
long commandTerm) throws IgniteInternalException {
+private boolean handleFinishTxCommand(FinishTxCommand cmd, long
commandIndex, long commandTerm) throws IgniteInternalException {
Review Comment:
Fixed.
--
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
[GitHub] [ignite-3] sergeyuttsel commented on a diff in pull request #1503: IGNITE-18324 Calling tx.rollback() or tx.commit() is not failed for committed transaction.
sergeyuttsel commented on code in PR #1503:
URL: https://github.com/apache/ignite-3/pull/1503#discussion_r1082415745
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadWriteTransactionImpl.java:
##
@@ -88,8 +93,12 @@ public IgniteBiTuple
enlist(ReplicationGroupId replicationGro
/** {@inheritDoc} */
@Override
protected CompletableFuture finish(boolean commit) {
Review Comment:
I've updated java doc.
--
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
[GitHub] [ignite-3] sergeyuttsel commented on a diff in pull request #1503: IGNITE-18324 Calling tx.rollback() or tx.commit() is not failed for committed transaction.
sergeyuttsel commented on code in PR #1503:
URL: https://github.com/apache/ignite-3/pull/1503#discussion_r1082415277
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadWriteTransactionImpl.java:
##
@@ -126,10 +135,14 @@ protected CompletableFuture finish(boolean commit) {
id()
);
} else {
-return CompletableFuture.completedFuture(null);
+return completedFuture(null);
}
}
);
+
+mainFinishFut.handle((res, e) -> finishFut.get().complete(null));
+
+return mainFinishFut;
Review Comment:
We want to get an original future on the first invocation. It can be
completed exceptionally.
--
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
[GitHub] [ignite-3] sergeyuttsel commented on a diff in pull request #1503: IGNITE-18324 Calling tx.rollback() or tx.commit() is not failed for committed transaction.
sergeyuttsel commented on code in PR #1503:
URL: https://github.com/apache/ignite-3/pull/1503#discussion_r1082413152
##
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadWriteTransactionImpl.java:
##
@@ -88,8 +93,12 @@ public IgniteBiTuple
enlist(ReplicationGroupId replicationGro
/** {@inheritDoc} */
@Override
protected CompletableFuture finish(boolean commit) {
+if (!finishFut.compareAndSet(null, new CompletableFuture<>())) {
+return finishFut.get();
+}
+
// TODO: https://issues.apache.org/jira/browse/IGNITE-17688 Add proper
exception handling.
-return CompletableFuture
+CompletableFuture mainFinishFut = CompletableFuture
Review Comment:
For me it is good. And I don't want to wrack my brain :)
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
korlov42 commented on code in PR #1524:
URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1082404654
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/SortAggregateNode.java:
##
@@ -76,7 +75,6 @@ public SortAggregateNode(
Comparator comp
) {
super(ctx);
-assert Objects.nonNull(comp);
Review Comment:
I would prefer to leave this assertion as is.
Without this assertion, a situation in which we would have previously
obtained either an assertion error or an NPE would now lead to the wrong result.
Perhaps, we could cover single group case by providing comparator returning
0 every time, WDYT?
--
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
[GitHub] [ignite] ivandasch merged pull request #10490: IGNITE-18584 Enlarge compression buffer in order to provide sufficient space for compressors.
ivandasch merged PR #10490: URL: https://github.com/apache/ignite/pull/10490 -- 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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082360088
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageWriteHandler.java:
##
@@ -0,0 +1,245 @@
+/*
+ * 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.metastorage.server.raft;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.GetAndPutAllCommand;
+import org.apache.ignite.internal.metastorage.command.GetAndPutCommand;
+import org.apache.ignite.internal.metastorage.command.GetAndRemoveAllCommand;
+import org.apache.ignite.internal.metastorage.command.GetAndRemoveCommand;
+import org.apache.ignite.internal.metastorage.command.InvokeCommand;
+import
org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.MultiInvokeCommand;
+import org.apache.ignite.internal.metastorage.command.MultipleEntryResponse;
+import org.apache.ignite.internal.metastorage.command.PutAllCommand;
+import org.apache.ignite.internal.metastorage.command.PutCommand;
+import org.apache.ignite.internal.metastorage.command.RemoveAllCommand;
+import org.apache.ignite.internal.metastorage.command.RemoveCommand;
+import org.apache.ignite.internal.metastorage.command.SingleEntryResponse;
+import
org.apache.ignite.internal.metastorage.command.info.CompoundConditionInfo;
+import org.apache.ignite.internal.metastorage.command.info.ConditionInfo;
+import org.apache.ignite.internal.metastorage.command.info.IfInfo;
+import org.apache.ignite.internal.metastorage.command.info.OperationInfo;
+import org.apache.ignite.internal.metastorage.command.info.SimpleConditionInfo;
+import org.apache.ignite.internal.metastorage.command.info.StatementInfo;
+import org.apache.ignite.internal.metastorage.command.info.UpdateInfo;
+import org.apache.ignite.internal.metastorage.dsl.CompoundConditionType;
+import org.apache.ignite.internal.metastorage.dsl.ConditionType;
+import org.apache.ignite.internal.metastorage.dsl.Operation;
+import org.apache.ignite.internal.metastorage.dsl.StatementResult;
+import org.apache.ignite.internal.metastorage.dsl.Update;
+import org.apache.ignite.internal.metastorage.server.AndCondition;
+import org.apache.ignite.internal.metastorage.server.Condition;
+import org.apache.ignite.internal.metastorage.server.ExistenceCondition;
+import org.apache.ignite.internal.metastorage.server.If;
+import org.apache.ignite.internal.metastorage.server.KeyValueStorage;
+import org.apache.ignite.internal.metastorage.server.OrCondition;
+import org.apache.ignite.internal.metastorage.server.RevisionCondition;
+import org.apache.ignite.internal.metastorage.server.Statement;
+import org.apache.ignite.internal.metastorage.server.TombstoneCondition;
+import org.apache.ignite.internal.metastorage.server.ValueCondition;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.CommandClosure;
+
+/**
+ * Class containing some common logic for Meta Storage Raft group listeners.
+ */
+class MetaStorageWriteHandler {
+private final MetaStorageCommandsFactory commandsFactory = new
MetaStorageCommandsFactory();
+
+private final KeyValueStorage storage;
+
+MetaStorageWriteHandler(KeyValueStorage storage) {
+this.storage = storage;
+}
+
+/**
+ * Tries to process a {@link WriteCommand}, returning {@code true} if the
command has been successfully processed or {@code false}
+ * if the command requires external processing.
+ */
+boolean handleWriteCommand(CommandClosure clo) {
+WriteCommand command = clo.command();
+
+if (command instanceof PutCommand) {
+PutCommand putCmd = (PutCommand) command;
+
+storage.put(putCmd.key(), putCmd.value());
+
+clo.result(null);
+} else if (command instanceof GetAndPutCommand) {
+GetAndPutCommand getAndPutCmd = (GetAndPutCommand) command;
+
+Entry e = storage.getAndPut(getAndPutCmd.key(),
getAndPutCm
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082359287
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageLearnerListener.java:
##
@@ -0,0 +1,73 @@
+/*
+ * 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.metastorage.server.raft;
+
+import java.nio.file.Path;
+import java.util.Iterator;
+import java.util.function.Consumer;
+import org.apache.ignite.internal.metastorage.server.KeyValueStorage;
+import org.apache.ignite.internal.raft.ReadCommand;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.CommandClosure;
+import org.apache.ignite.internal.raft.service.RaftGroupListener;
+
+/**
+ * Meta Storage Raft group listener for learner nodes. These nodes ignore read
and cursor-related commands.
+ */
+public class MetaStorageLearnerListener implements RaftGroupListener {
+private final KeyValueStorage storage;
+
+private final MetaStorageWriteHandler writeHandler;
+
+public MetaStorageLearnerListener(KeyValueStorage storage) {
+this.storage = storage;
+this.writeHandler = new MetaStorageWriteHandler(storage);
+}
+
+@Override
+public void onRead(Iterator> iterator) {
+throw new UnsupportedOperationException("Reads should not be sent to
learners");
+}
+
+@Override
+public void onWrite(Iterator> iter) {
+while (iter.hasNext()) {
+CommandClosure clo = iter.next();
+
+if (!writeHandler.handleWriteCommand(clo)) {
+// Ignore all commands that are not handled by the
writeHandler.
+clo.result(null);
Review Comment:
I don't understand the question, sorry. This is a closure for a particular
command, which we are skipping if it was not recognized by the write handler.
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082356978
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java:
##
@@ -842,11 +753,9 @@ public boolean hasNext() {
}
try {
-try {
-return innerIterFut.thenApply(Iterator::hasNext).get();
-} catch (InterruptedException | ExecutionException e) {
-throw new MetaStorageException(CURSOR_EXECUTION_ERR, e);
-}
+return innerCursorFut.thenApply(Iterator::hasNext).get();
+} catch (InterruptedException | ExecutionException e) {
+throw new MetaStorageException(CURSOR_EXECUTION_ERR, e);
Review Comment:
Can you clarify please? What catches are you talking about?
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082352267
##
modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageWatchTest.java:
##
@@ -0,0 +1,339 @@
+/*
+ * 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.metastorage.impl;
+
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
+import static
org.apache.ignite.utils.ClusterServiceTestUtils.findLocalAddresses;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.BiConsumer;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import
org.apache.ignite.internal.cluster.management.ClusterManagementGroupManager;
+import
org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
+import
org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.metastorage.MetaStorageManager;
+import org.apache.ignite.internal.metastorage.WatchEvent;
+import org.apache.ignite.internal.metastorage.WatchListener;
+import org.apache.ignite.internal.metastorage.dsl.Conditions;
+import org.apache.ignite.internal.metastorage.dsl.Operations;
+import
org.apache.ignite.internal.metastorage.server.persistence.RocksDbKeyValueStorage;
+import org.apache.ignite.internal.raft.Loza;
+import org.apache.ignite.internal.raft.RaftManager;
+import org.apache.ignite.internal.raft.configuration.RaftConfiguration;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.vault.VaultManager;
+import org.apache.ignite.lang.ByteArray;
+import org.apache.ignite.lang.NodeStoppingException;
+import org.apache.ignite.network.ClusterService;
+import org.apache.ignite.network.NetworkAddress;
+import org.apache.ignite.network.StaticNodeFinder;
+import org.apache.ignite.utils.ClusterServiceTestUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Tests for Meta Storage Watches.
+ */
+@ExtendWith(WorkDirectoryExtension.class)
+@ExtendWith(ConfigurationExtension.class)
+public class ItMetaStorageWatchTest {
+private static class Node {
+private final ClusterService clusterService;
+
+private final RaftManager raftManager;
+
+private final MetaStorageManager metaStorageManager;
+
+private final CompletableFuture> metaStorageNodesFuture =
new CompletableFuture<>();
+
+Node(ClusterService clusterService, RaftConfiguration
raftConfiguration, Path dataPath) {
+this.clusterService = clusterService;
+
+Path basePath = dataPath.resolve(name());
+
+this.raftManager = new Loza(
+clusterService,
+raftConfiguration,
+basePath.resolve("raft"),
+new HybridClockImpl()
+);
+
+var vaultManager = mock(VaultManager.class);
Review Comment:
This way I'll have to save the VaultManager as a component in order to
start/stop it properly, so this won't simplify anything
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1513: IGNITE-18163: Old-style join on different column types fails with ClassCastException
AMashenkov commented on code in PR #1513:
URL: https://github.com/apache/ignite-3/pull/1513#discussion_r1082350058
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/type/IgniteTypeSystem.java:
##
@@ -45,6 +45,8 @@ public int getMaxNumericPrecision() {
return Short.MAX_VALUE;
}
+
+
Review Comment:
```suggestion
```
--
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
[GitHub] [ignite-3] lowka commented on a diff in pull request #1528: IGNITE-18422: Sql. Match number of dynamic parameters with given parameters
lowka commented on code in PR #1528: URL: https://github.com/apache/ignite-3/pull/1528#discussion_r1082340129 ## modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java: ## @@ -251,6 +251,8 @@ public static Matcher containsAnyScan(final String schema, final String private String exactPlan; +private boolean executeExplainPlan = true; Review Comment: Yep. Going to fix this since it does not work anyway. -- 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
[GitHub] [ignite-3] ptupitsyn merged pull request #1554: IGNITE-18588 .NET: Fix BinaryTupleReader behavior on payload size mismatch
ptupitsyn merged PR #1554: URL: https://github.com/apache/ignite-3/pull/1554 -- 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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1528: IGNITE-18422: Sql. Match number of dynamic parameters with given parameters
AMashenkov commented on code in PR #1528: URL: https://github.com/apache/ignite-3/pull/1528#discussion_r1082320651 ## modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java: ## @@ -251,6 +251,8 @@ public static Matcher containsAnyScan(final String schema, final String private String exactPlan; +private boolean executeExplainPlan = true; Review Comment: It looks unclear for me. If `executeExplainPlan` is true, then will the query itself be excecuted, or just EXPLAIN for the query? Maybe `skipExplan` will be more precise name. ## modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java: ## @@ -251,6 +251,8 @@ public static Matcher containsAnyScan(final String schema, final String private String exactPlan; +private boolean executeExplainPlan = true; Review Comment: It looks unclear for me. If `executeExplainPlan` is true, then will the query itself be excecuted, or just EXPLAIN for the query? Maybe `skipExplain` will be more precise name. -- 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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1528: IGNITE-18422: Sql. Match number of dynamic parameters with given parameters
AMashenkov commented on code in PR #1528:
URL: https://github.com/apache/ignite-3/pull/1528#discussion_r1082316721
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java:
##
@@ -363,27 +365,54 @@ public QueryChecker planEquals(String plan) {
return this;
}
+/**
+ * Specifies whether to execute {@code EXPLAIN PLAN FOR} statement before
running a query or not.
+ * The default value is {@code true}.
+ *
+ * If the value is set to {@code false} but either plan or at least one
plan matcher is specified {@link #check()}
+ * throws {@link IllegalStateException}.
+ *
+ *
+ * @param value whether to execute EXPLAIN PLAN statement or not.
+ * @return this
Review Comment:
```suggestion
*
* @param value whether to execute EXPLAIN PLAN statement or not.
* @return {@code this}
* @throws IllegalStateException If the value is set to {@code false}
but either plan or at least one plan matcher is specified {@link #check()}
```
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1528: IGNITE-18422: Sql. Match number of dynamic parameters with given parameters
AMashenkov commented on code in PR #1528:
URL: https://github.com/apache/ignite-3/pull/1528#discussion_r1082316721
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java:
##
@@ -363,27 +365,54 @@ public QueryChecker planEquals(String plan) {
return this;
}
+/**
+ * Specifies whether to execute {@code EXPLAIN PLAN FOR} statement before
running a query or not.
+ * The default value is {@code true}.
+ *
+ * If the value is set to {@code false} but either plan or at least one
plan matcher is specified {@link #check()}
+ * throws {@link IllegalStateException}.
+ *
+ *
+ * @param value whether to execute EXPLAIN PLAN statement or not.
+ * @return this
Review Comment:
```suggestion
*
* @param value whether to execute EXPLAIN PLAN statement or not.
* @return this
* @throws IllegalStateException If the value is set to {@code false}
but either plan or at least one plan matcher is specified {@link #check()}
```
--
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
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1528: IGNITE-18422: Sql. Match number of dynamic parameters with given parameters
AMashenkov commented on code in PR #1528:
URL: https://github.com/apache/ignite-3/pull/1528#discussion_r1082316721
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java:
##
@@ -363,27 +365,54 @@ public QueryChecker planEquals(String plan) {
return this;
}
+/**
+ * Specifies whether to execute {@code EXPLAIN PLAN FOR} statement before
running a query or not.
+ * The default value is {@code true}.
+ *
+ * If the value is set to {@code false} but either plan or at least one
plan matcher is specified {@link #check()}
+ * throws {@link IllegalStateException}.
+ *
+ *
+ * @param value whether to execute EXPLAIN PLAN statement or not.
+ * @return this
Review Comment:
```suggestion
*
* @param value whether to execute EXPLAIN PLAN statement or not.
* @return this
* @throws If the value is set to {@code false} but either plan or at
least one plan matcher is specified {@link #check()}
```
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082315255
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageWriteHandler.java:
##
@@ -0,0 +1,245 @@
+/*
+ * 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.metastorage.server.raft;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.GetAndPutAllCommand;
+import org.apache.ignite.internal.metastorage.command.GetAndPutCommand;
+import org.apache.ignite.internal.metastorage.command.GetAndRemoveAllCommand;
+import org.apache.ignite.internal.metastorage.command.GetAndRemoveCommand;
+import org.apache.ignite.internal.metastorage.command.InvokeCommand;
+import
org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.MultiInvokeCommand;
+import org.apache.ignite.internal.metastorage.command.MultipleEntryResponse;
+import org.apache.ignite.internal.metastorage.command.PutAllCommand;
+import org.apache.ignite.internal.metastorage.command.PutCommand;
+import org.apache.ignite.internal.metastorage.command.RemoveAllCommand;
+import org.apache.ignite.internal.metastorage.command.RemoveCommand;
+import org.apache.ignite.internal.metastorage.command.SingleEntryResponse;
+import
org.apache.ignite.internal.metastorage.command.info.CompoundConditionInfo;
+import org.apache.ignite.internal.metastorage.command.info.ConditionInfo;
+import org.apache.ignite.internal.metastorage.command.info.IfInfo;
+import org.apache.ignite.internal.metastorage.command.info.OperationInfo;
+import org.apache.ignite.internal.metastorage.command.info.SimpleConditionInfo;
+import org.apache.ignite.internal.metastorage.command.info.StatementInfo;
+import org.apache.ignite.internal.metastorage.command.info.UpdateInfo;
+import org.apache.ignite.internal.metastorage.dsl.CompoundConditionType;
+import org.apache.ignite.internal.metastorage.dsl.ConditionType;
+import org.apache.ignite.internal.metastorage.dsl.Operation;
+import org.apache.ignite.internal.metastorage.dsl.StatementResult;
+import org.apache.ignite.internal.metastorage.dsl.Update;
+import org.apache.ignite.internal.metastorage.server.AndCondition;
+import org.apache.ignite.internal.metastorage.server.Condition;
+import org.apache.ignite.internal.metastorage.server.ExistenceCondition;
+import org.apache.ignite.internal.metastorage.server.If;
+import org.apache.ignite.internal.metastorage.server.KeyValueStorage;
+import org.apache.ignite.internal.metastorage.server.OrCondition;
+import org.apache.ignite.internal.metastorage.server.RevisionCondition;
+import org.apache.ignite.internal.metastorage.server.Statement;
+import org.apache.ignite.internal.metastorage.server.TombstoneCondition;
+import org.apache.ignite.internal.metastorage.server.ValueCondition;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.CommandClosure;
+
+/**
+ * Class containing some common logic for Meta Storage Raft group listeners.
+ */
+class MetaStorageWriteHandler {
+private final MetaStorageCommandsFactory commandsFactory = new
MetaStorageCommandsFactory();
+
+private final KeyValueStorage storage;
+
+MetaStorageWriteHandler(KeyValueStorage storage) {
+this.storage = storage;
+}
+
+/**
+ * Tries to process a {@link WriteCommand}, returning {@code true} if the
command has been successfully processed or {@code false}
+ * if the command requires external processing.
+ */
+boolean handleWriteCommand(CommandClosure clo) {
+WriteCommand command = clo.command();
+
+if (command instanceof PutCommand) {
+PutCommand putCmd = (PutCommand) command;
+
+storage.put(putCmd.key(), putCmd.value());
+
+clo.result(null);
+} else if (command instanceof GetAndPutCommand) {
+GetAndPutCommand getAndPutCmd = (GetAndPutCommand) command;
+
+Entry e = storage.getAndPut(getAndPutCmd.key(),
getAndPutCm
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082314883
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageWriteHandler.java:
##
@@ -0,0 +1,245 @@
+/*
+ * 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.metastorage.server.raft;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.GetAndPutAllCommand;
+import org.apache.ignite.internal.metastorage.command.GetAndPutCommand;
+import org.apache.ignite.internal.metastorage.command.GetAndRemoveAllCommand;
+import org.apache.ignite.internal.metastorage.command.GetAndRemoveCommand;
+import org.apache.ignite.internal.metastorage.command.InvokeCommand;
+import
org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.MultiInvokeCommand;
+import org.apache.ignite.internal.metastorage.command.MultipleEntryResponse;
+import org.apache.ignite.internal.metastorage.command.PutAllCommand;
+import org.apache.ignite.internal.metastorage.command.PutCommand;
+import org.apache.ignite.internal.metastorage.command.RemoveAllCommand;
+import org.apache.ignite.internal.metastorage.command.RemoveCommand;
+import org.apache.ignite.internal.metastorage.command.SingleEntryResponse;
+import
org.apache.ignite.internal.metastorage.command.info.CompoundConditionInfo;
+import org.apache.ignite.internal.metastorage.command.info.ConditionInfo;
+import org.apache.ignite.internal.metastorage.command.info.IfInfo;
+import org.apache.ignite.internal.metastorage.command.info.OperationInfo;
+import org.apache.ignite.internal.metastorage.command.info.SimpleConditionInfo;
+import org.apache.ignite.internal.metastorage.command.info.StatementInfo;
+import org.apache.ignite.internal.metastorage.command.info.UpdateInfo;
+import org.apache.ignite.internal.metastorage.dsl.CompoundConditionType;
+import org.apache.ignite.internal.metastorage.dsl.ConditionType;
+import org.apache.ignite.internal.metastorage.dsl.Operation;
+import org.apache.ignite.internal.metastorage.dsl.StatementResult;
+import org.apache.ignite.internal.metastorage.dsl.Update;
+import org.apache.ignite.internal.metastorage.server.AndCondition;
+import org.apache.ignite.internal.metastorage.server.Condition;
+import org.apache.ignite.internal.metastorage.server.ExistenceCondition;
+import org.apache.ignite.internal.metastorage.server.If;
+import org.apache.ignite.internal.metastorage.server.KeyValueStorage;
+import org.apache.ignite.internal.metastorage.server.OrCondition;
+import org.apache.ignite.internal.metastorage.server.RevisionCondition;
+import org.apache.ignite.internal.metastorage.server.Statement;
+import org.apache.ignite.internal.metastorage.server.TombstoneCondition;
+import org.apache.ignite.internal.metastorage.server.ValueCondition;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.CommandClosure;
+
+/**
+ * Class containing some common logic for Meta Storage Raft group listeners.
+ */
+class MetaStorageWriteHandler {
+private final MetaStorageCommandsFactory commandsFactory = new
MetaStorageCommandsFactory();
+
+private final KeyValueStorage storage;
+
+MetaStorageWriteHandler(KeyValueStorage storage) {
+this.storage = storage;
+}
+
+/**
+ * Tries to process a {@link WriteCommand}, returning {@code true} if the
command has been successfully processed or {@code false}
+ * if the command requires external processing.
+ */
+boolean handleWriteCommand(CommandClosure clo) {
+WriteCommand command = clo.command();
+
+if (command instanceof PutCommand) {
Review Comment:
I don't know, maybe because creating a lot of classes with a couple of lines
of code is an overkill.
--
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.apac
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082313704
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java:
##
@@ -159,54 +135,164 @@ public MetaStorageManagerImpl(
this.storage = storage;
}
-private CompletableFuture
initializeMetaStorage(Set metaStorageNodes) {
+private CompletableFuture
initializeMetaStorage(Set metaStorageNodes) {
ClusterNode thisNode = clusterService.topologyService().localMember();
-PeersAndLearners configuration =
PeersAndLearners.fromConsistentIds(metaStorageNodes);
-
-Peer localPeer = configuration.peer(thisNode.name());
+String thisNodeName = thisNode.name();
CompletableFuture raftServiceFuture;
try {
-if (localPeer == null) {
-raftServiceFuture =
raftMgr.startRaftGroupService(MetastorageGroupId.INSTANCE, configuration);
-} else {
-clusterService.topologyService().addEventHandler(new
TopologyEventHandler() {
-@Override
-public void onDisappeared(ClusterNode member) {
-metaStorageSvcFut.thenAccept(svc ->
svc.closeCursors(member.id()));
-}
-});
+// We need to configure the replication protocol differently
whether this node is a synchronous or asynchronous replica.
+if (metaStorageNodes.contains(thisNodeName)) {
+PeersAndLearners configuration =
PeersAndLearners.fromConsistentIds(metaStorageNodes);
+
+Peer localPeer = configuration.peer(thisNodeName);
-storage.start();
+assert localPeer != null;
raftServiceFuture = raftMgr.startRaftGroupNode(
new RaftNodeId(MetastorageGroupId.INSTANCE, localPeer),
configuration,
new MetaStorageListener(storage),
+new RaftGroupEventsListener() {
+@Override
+public void onLeaderElected(long term) {
+// TODO: add listener to remove learners when
they leave Logical Topology
+// see
https://issues.apache.org/jira/browse/IGNITE-18554
+
+registerTopologyEventListener();
+
+// Update the configuration immediately in
case we missed some updates.
+
addLearners(clusterService.topologyService().allMembers());
+}
+
+@Override
+public void
onNewPeersConfigurationApplied(PeersAndLearners configuration) {
+}
+
+@Override
+public void onReconfigurationError(Status status,
PeersAndLearners configuration, long term) {
+}
+}
+);
+} else {
+PeersAndLearners configuration =
PeersAndLearners.fromConsistentIds(metaStorageNodes, Set.of(thisNodeName));
+
+Peer localPeer = configuration.learner(thisNodeName);
+
+assert localPeer != null;
+
+raftServiceFuture = raftMgr.startRaftGroupNode(
+new RaftNodeId(MetastorageGroupId.INSTANCE, localPeer),
+configuration,
+new MetaStorageLearnerListener(storage),
RaftGroupEventsListener.noopLsnr
);
}
} catch (NodeStoppingException e) {
return CompletableFuture.failedFuture(e);
}
-return raftServiceFuture.thenApply(service -> new
MetaStorageServiceImpl(service, thisNode.id(), thisNode.name()));
+return raftServiceFuture.thenApply(raftService -> new
MetaStorageServiceImpl(raftService, thisNode));
+}
+
+private void registerTopologyEventListener() {
+clusterService.topologyService().addEventHandler(new
TopologyEventHandler() {
+@Override
+public void onAppeared(ClusterNode member) {
+addLearners(List.of(member));
+}
+
+@Override
+public void onDisappeared(ClusterNode member) {
+metaStorageSvcFut.thenAccept(service ->
isCurrentNodeLeader(service.raftGroupService())
+.thenAccept(isLeader -> {
+if (isLeader) {
+service.closeCursors(member.id());
+}
+}));
+}
+});
+}
+
+private void addLearners(Collection nodes) {
+if (!busyLock.enterBusy()) {
+LOG.info("Skipping Meta Storage confi
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082313013
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java:
##
@@ -159,54 +135,164 @@ public MetaStorageManagerImpl(
this.storage = storage;
}
-private CompletableFuture
initializeMetaStorage(Set metaStorageNodes) {
+private CompletableFuture
initializeMetaStorage(Set metaStorageNodes) {
ClusterNode thisNode = clusterService.topologyService().localMember();
-PeersAndLearners configuration =
PeersAndLearners.fromConsistentIds(metaStorageNodes);
-
-Peer localPeer = configuration.peer(thisNode.name());
+String thisNodeName = thisNode.name();
CompletableFuture raftServiceFuture;
try {
-if (localPeer == null) {
-raftServiceFuture =
raftMgr.startRaftGroupService(MetastorageGroupId.INSTANCE, configuration);
-} else {
-clusterService.topologyService().addEventHandler(new
TopologyEventHandler() {
-@Override
-public void onDisappeared(ClusterNode member) {
-metaStorageSvcFut.thenAccept(svc ->
svc.closeCursors(member.id()));
-}
-});
+// We need to configure the replication protocol differently
whether this node is a synchronous or asynchronous replica.
+if (metaStorageNodes.contains(thisNodeName)) {
+PeersAndLearners configuration =
PeersAndLearners.fromConsistentIds(metaStorageNodes);
+
+Peer localPeer = configuration.peer(thisNodeName);
-storage.start();
+assert localPeer != null;
raftServiceFuture = raftMgr.startRaftGroupNode(
new RaftNodeId(MetastorageGroupId.INSTANCE, localPeer),
configuration,
new MetaStorageListener(storage),
+new RaftGroupEventsListener() {
+@Override
+public void onLeaderElected(long term) {
+// TODO: add listener to remove learners when
they leave Logical Topology
+// see
https://issues.apache.org/jira/browse/IGNITE-18554
+
+registerTopologyEventListener();
+
+// Update the configuration immediately in
case we missed some updates.
+
addLearners(clusterService.topologyService().allMembers());
+}
+
+@Override
+public void
onNewPeersConfigurationApplied(PeersAndLearners configuration) {
+}
+
+@Override
+public void onReconfigurationError(Status status,
PeersAndLearners configuration, long term) {
+}
+}
+);
+} else {
+PeersAndLearners configuration =
PeersAndLearners.fromConsistentIds(metaStorageNodes, Set.of(thisNodeName));
+
+Peer localPeer = configuration.learner(thisNodeName);
+
+assert localPeer != null;
+
+raftServiceFuture = raftMgr.startRaftGroupNode(
+new RaftNodeId(MetastorageGroupId.INSTANCE, localPeer),
+configuration,
+new MetaStorageLearnerListener(storage),
RaftGroupEventsListener.noopLsnr
);
}
} catch (NodeStoppingException e) {
return CompletableFuture.failedFuture(e);
}
-return raftServiceFuture.thenApply(service -> new
MetaStorageServiceImpl(service, thisNode.id(), thisNode.name()));
+return raftServiceFuture.thenApply(raftService -> new
MetaStorageServiceImpl(raftService, thisNode));
+}
+
+private void registerTopologyEventListener() {
+clusterService.topologyService().addEventHandler(new
TopologyEventHandler() {
+@Override
+public void onAppeared(ClusterNode member) {
+addLearners(List.of(member));
+}
+
+@Override
+public void onDisappeared(ClusterNode member) {
+metaStorageSvcFut.thenAccept(service ->
isCurrentNodeLeader(service.raftGroupService())
Review Comment:
yes, this implementation is buggy, I will fix it when we are going to move
to reactive cursors
--
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 contac
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082312367
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageManagerImpl.java:
##
@@ -159,54 +135,164 @@ public MetaStorageManagerImpl(
this.storage = storage;
}
-private CompletableFuture
initializeMetaStorage(Set metaStorageNodes) {
+private CompletableFuture
initializeMetaStorage(Set metaStorageNodes) {
ClusterNode thisNode = clusterService.topologyService().localMember();
-PeersAndLearners configuration =
PeersAndLearners.fromConsistentIds(metaStorageNodes);
-
-Peer localPeer = configuration.peer(thisNode.name());
+String thisNodeName = thisNode.name();
CompletableFuture raftServiceFuture;
try {
-if (localPeer == null) {
-raftServiceFuture =
raftMgr.startRaftGroupService(MetastorageGroupId.INSTANCE, configuration);
-} else {
-clusterService.topologyService().addEventHandler(new
TopologyEventHandler() {
-@Override
-public void onDisappeared(ClusterNode member) {
-metaStorageSvcFut.thenAccept(svc ->
svc.closeCursors(member.id()));
-}
-});
+// We need to configure the replication protocol differently
whether this node is a synchronous or asynchronous replica.
+if (metaStorageNodes.contains(thisNodeName)) {
+PeersAndLearners configuration =
PeersAndLearners.fromConsistentIds(metaStorageNodes);
+
+Peer localPeer = configuration.peer(thisNodeName);
-storage.start();
+assert localPeer != null;
raftServiceFuture = raftMgr.startRaftGroupNode(
new RaftNodeId(MetastorageGroupId.INSTANCE, localPeer),
configuration,
new MetaStorageListener(storage),
+new RaftGroupEventsListener() {
+@Override
+public void onLeaderElected(long term) {
+// TODO: add listener to remove learners when
they leave Logical Topology
+// see
https://issues.apache.org/jira/browse/IGNITE-18554
+
+registerTopologyEventListener();
+
+// Update the configuration immediately in
case we missed some updates.
+
addLearners(clusterService.topologyService().allMembers());
+}
+
+@Override
+public void
onNewPeersConfigurationApplied(PeersAndLearners configuration) {
Review Comment:
This PR is huge already, I don't want to mess with unrelated parts
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082311805
##
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##
@@ -741,39 +742,27 @@ private void initDataNodesFromVaultManager() {
}
try {
-// TODO: Remove this call as part of
https://issues.apache.org/jira/browse/IGNITE-18397
-vaultMgr.get(MetaStorageManagerImpl.APPLIED_REV)
-.thenApply(appliedRevision -> appliedRevision == null ? 0L
: bytesToLong(appliedRevision.value()))
-.thenAccept(vaultAppliedRevision -> {
+long appliedRevision = metaStorageManager.appliedRevision();
+
+vaultMgr.get(zonesLogicalTopologyKey())
Review Comment:
Can you clarify please?
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
sashapolo commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1082310535
##
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/server/KeyValueStorage.java:
##
@@ -220,32 +222,54 @@ public interface KeyValueStorage extends
ManuallyCloseable {
* @param keyFrom Start key of range (inclusive).
* @param keyTo Last key of range (exclusive).
* @param rev Start revision number.
- * @return Cursor by update events.
*/
-Cursor watch(byte[] keyFrom, byte @Nullable [] keyTo, long
rev);
+void watchRange(byte[] keyFrom, byte @Nullable [] keyTo, long rev,
WatchListener listener);
/**
- * Creates subscription on updates of entries corresponding to the given
keys range (where upper bound is unlimited) and starting from
- * the given revision number.
+ * Registers a watch listener by a key prefix.
*
- * @param key Start key of range (inclusive).
- * @param rev Start revision number.
- * @return Cursor by update events.
+ * @param prefix Prefix to listen to.
+ * @param rev Starting Meta Storage revision.
+ * @param listener Listener which will be notified for each update.
*/
-Cursor watch(byte[] key, long rev);
+void watchPrefix(byte[] prefix, long rev, WatchListener listener);
/**
- * Creates subscription on updates of entries corresponding to the given
keys collection and starting from the given revision number.
+ * Registers a watch listener for the provided key.
*
- * @param keys Collection of keys
- * @param rev Start revision number.
- * @return Cursor by update events.
+ * @param key Meta Storage key.
+ * @param rev Starting Meta Storage revision.
+ * @param listener Listener which will be notified for each update.
*/
-Cursor watch(Collection keys, long rev);
+void watchExact(byte[] key, long rev, WatchListener listener);
+
+/**
+ * Registers a watch listener for the provided keys.
+ *
+ * @param keys Meta Storage keys.
+ * @param rev Starting Meta Storage revision.
+ * @param listener Listener which will be notified for each update.
+ */
+void watchExact(Collection keys, long rev, WatchListener listener);
+
+/**
+ * Starts all registered watches.
+ *
+ * Before calling this method, watches will not receive any updates.
+ *
+ * @param revisionCallback Callback that will be invoked after all watches
of a particular revision are processed, with the revision
+ * as its argument.
+ */
+void startWatches(LongConsumer revisionCallback);
+
+/**
+ * Unregisters a watch listener.
+ */
+void removeWatch(WatchListener listener);
/**
* Compacts storage (removes tombstones).
- * TODO: IGNITE-16444 Сorrect compaction for Metastorage.
+ * TODO: IGNITE-16444 Correct compaction for Metastorage.
Review Comment:
I think yes
--
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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
korlov42 commented on code in PR #1524: URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1082304386 ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rule/SortAggregateConverterRule.java: ## @@ -36,6 +36,7 @@ import org.apache.ignite.internal.sql.engine.trait.IgniteDistributions; import org.apache.ignite.internal.sql.engine.trait.TraitUtils; import org.apache.ignite.internal.sql.engine.util.HintUtils; +import org.checkerframework.checker.nullness.qual.Nullable; Review Comment: the same -- 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
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1524: IGNITE-18464 Sql. Colocated sort aggregates need to compose a plans with additional sort
korlov42 commented on code in PR #1524: URL: https://github.com/apache/ignite-3/pull/1524#discussion_r1082303971 ## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/rule/AbstractIgniteConverterRule.java: ## @@ -24,6 +24,7 @@ import org.apache.calcite.rel.convert.ConverterRule; import org.apache.calcite.rel.metadata.RelMetadataQuery; import org.apache.ignite.internal.sql.engine.rel.IgniteConvention; +import org.checkerframework.checker.nullness.qual.Nullable; Review Comment: I believe it has to be `org.jetbrains.annotations` -- 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
[GitHub] [ignite-3] ibessonov commented on a diff in pull request #1490: IGNITE-18397 Rework Watches based on Raft Learners
ibessonov commented on code in PR #1490:
URL: https://github.com/apache/ignite-3/pull/1490#discussion_r1080883366
##
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##
@@ -741,39 +742,27 @@ private void initDataNodesFromVaultManager() {
}
try {
-// TODO: Remove this call as part of
https://issues.apache.org/jira/browse/IGNITE-18397
-vaultMgr.get(MetaStorageManagerImpl.APPLIED_REV)
-.thenApply(appliedRevision -> appliedRevision == null ? 0L
: bytesToLong(appliedRevision.value()))
-.thenAccept(vaultAppliedRevision -> {
+long appliedRevision = metaStorageManager.appliedRevision();
+
+vaultMgr.get(zonesLogicalTopologyKey())
Review Comment:
Any idea why Vault is asynchronous?
##
modules/metastorage/src/integrationTest/java/org/apache/ignite/internal/metastorage/impl/ItMetaStorageWatchTest.java:
##
@@ -0,0 +1,339 @@
+/*
+ * 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.metastorage.impl;
+
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
+import static
org.apache.ignite.utils.ClusterServiceTestUtils.findLocalAddresses;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.function.BiConsumer;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import
org.apache.ignite.internal.cluster.management.ClusterManagementGroupManager;
+import
org.apache.ignite.internal.configuration.testframework.ConfigurationExtension;
+import
org.apache.ignite.internal.configuration.testframework.InjectConfiguration;
+import org.apache.ignite.internal.hlc.HybridClockImpl;
+import org.apache.ignite.internal.metastorage.MetaStorageManager;
+import org.apache.ignite.internal.metastorage.WatchEvent;
+import org.apache.ignite.internal.metastorage.WatchListener;
+import org.apache.ignite.internal.metastorage.dsl.Conditions;
+import org.apache.ignite.internal.metastorage.dsl.Operations;
+import
org.apache.ignite.internal.metastorage.server.persistence.RocksDbKeyValueStorage;
+import org.apache.ignite.internal.raft.Loza;
+import org.apache.ignite.internal.raft.RaftManager;
+import org.apache.ignite.internal.raft.configuration.RaftConfiguration;
+import org.apache.ignite.internal.testframework.WorkDirectory;
+import org.apache.ignite.internal.testframework.WorkDirectoryExtension;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.vault.VaultManager;
+import org.apache.ignite.lang.ByteArray;
+import org.apache.ignite.lang.NodeStoppingException;
+import org.apache.ignite.network.ClusterService;
+import org.apache.ignite.network.NetworkAddress;
+import org.apache.ignite.network.StaticNodeFinder;
+import org.apache.ignite.utils.ClusterServiceTestUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Tests for Meta Storage Watches.
+ */
+@ExtendWith(WorkDirectoryExtension.class)
+@ExtendWith(ConfigurationExtension.class)
+public class ItMetaStorageWatchTest {
+private static class Node {
+private final ClusterService clusterService;
+
+private final RaftManager raftManager;
+
+private final MetaStorageManager metaStorageManager;
+
+private final CompletableFuture> metaStorageNodesFuture =
new Completabl
[GitHub] [ignite-3] korlov42 commented on a diff in pull request #1550: IGNITE-18585 Sql. Introduce cache for serialized RelNodes representation
korlov42 commented on code in PR #1550:
URL: https://github.com/apache/ignite-3/pull/1550#discussion_r1082287428
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ExecutionServiceImpl.java:
##
@@ -83,6 +86,13 @@
* ExecutionServiceImpl. TODO Documentation
https://issues.apache.org/jira/browse/IGNITE-15859
*/
public class ExecutionServiceImpl implements ExecutionService,
TopologyEventHandler {
+private static final int CACHE_SIZE = 1024;
+
+private static final ConcurrentMap PHYS_NODES_REPR =
Caffeine.newBuilder()
Review Comment:
honestly speaking, I'm not sure this cache should be static. This can affect
the tests in an undesirable way, with no real benefit
--
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
[GitHub] [ignite-3] tkalkirill opened a new pull request, #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill opened a new pull request, #1556: URL: https://github.com/apache/ignite-3/pull/1556 https://issues.apache.org/jira/browse/IGNITE-18593 -- 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
