[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1562: IGNITE-18030 Integration of the new full rebalance API with IncomingSnapshotCopier
tkalkirill commented on code in PR #1562:
URL: https://github.com/apache/ignite-3/pull/1562#discussion_r1084898716
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/incoming/IncomingSnapshotCopier.java:
##
@@ -96,38 +102,37 @@ public IncomingSnapshotCopier(PartitionSnapshotStorage
partitionSnapshotStorage,
public void start() {
Executor executor =
partitionSnapshotStorage.getIncomingSnapshotsExecutor();
-LOG.info("Copier is started for the partition [partId={},
tableId={}]", partId(), tableId());
+LOG.info("Copier is started for the partition [{}]",
createPartitionInfo());
+
+joinFuture = new CompletableFuture<>();
-future = prepareMvPartitionStorageForRebalance()
-.thenCompose(unused ->
prepareTxStatePartitionStorageForRebalance(executor))
+rebalanceFuture = partitionSnapshotStorage.partition().startRebalance()
.thenCompose(unused -> {
ClusterNode snapshotSender =
getSnapshotSender(snapshotUri.nodeName);
if (snapshotSender == null) {
-LOG.error(
-"Snapshot sender not found [partId={},
tableId={}, nodeName={}]",
-partId(),
-tableId(),
-snapshotUri.nodeName
-);
-
-if (!isOk()) {
-setError(RaftError.UNKNOWN, "Sender node was not
found or it is offline");
-}
-
-return completedFuture(null);
+throw new StorageRebalanceException("Snapshot sender
not found: " + snapshotUri.nodeName);
}
return loadSnapshotMeta(snapshotSender)
.thenCompose(unused1 ->
loadSnapshotMvData(snapshotSender, executor))
-.thenCompose(unused1 ->
loadSnapshotTxData(snapshotSender, executor))
-.thenAcceptAsync(unused1 ->
updateLastAppliedIndexFromSnapshotMetaForStorages(), executor);
+.thenCompose(unused1 ->
loadSnapshotTxData(snapshotSender, executor));
+});
+
+rebalanceFuture.handle((unused, throwable) ->
completesRebalance(throwable))
Review Comment:
Type. fix it.
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/incoming/IncomingSnapshotCopier.java:
##
@@ -96,38 +102,37 @@ public IncomingSnapshotCopier(PartitionSnapshotStorage
partitionSnapshotStorage,
public void start() {
Executor executor =
partitionSnapshotStorage.getIncomingSnapshotsExecutor();
-LOG.info("Copier is started for the partition [partId={},
tableId={}]", partId(), tableId());
+LOG.info("Copier is started for the partition [{}]",
createPartitionInfo());
+
+joinFuture = new CompletableFuture<>();
-future = prepareMvPartitionStorageForRebalance()
-.thenCompose(unused ->
prepareTxStatePartitionStorageForRebalance(executor))
+rebalanceFuture = partitionSnapshotStorage.partition().startRebalance()
.thenCompose(unused -> {
ClusterNode snapshotSender =
getSnapshotSender(snapshotUri.nodeName);
if (snapshotSender == null) {
-LOG.error(
-"Snapshot sender not found [partId={},
tableId={}, nodeName={}]",
-partId(),
-tableId(),
-snapshotUri.nodeName
-);
-
-if (!isOk()) {
-setError(RaftError.UNKNOWN, "Sender node was not
found or it is offline");
-}
-
-return completedFuture(null);
+throw new StorageRebalanceException("Snapshot sender
not found: " + snapshotUri.nodeName);
}
return loadSnapshotMeta(snapshotSender)
.thenCompose(unused1 ->
loadSnapshotMvData(snapshotSender, executor))
-.thenCompose(unused1 ->
loadSnapshotTxData(snapshotSender, executor))
-.thenAcceptAsync(unused1 ->
updateLastAppliedIndexFromSnapshotMetaForStorages(), executor);
+.thenCompose(unused1 ->
loadSnapshotTxData(snapshotSender, executor));
+});
+
+rebalanceFuture.handle((unused, throwable) ->
completesRebalance(throwable))
+.thenCompose(Function.identity())
+.whenComplete((unused, throwable) -> {
+if (throwable == null) {
+joinFuture.complete(null);
Review Comment:
Fix it.
-
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1562: IGNITE-18030 Integration of the new full rebalance API with IncomingSnapshotCopier
tkalkirill commented on code in PR #1562:
URL: https://github.com/apache/ignite-3/pull/1562#discussion_r1084898581
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/incoming/IncomingSnapshotCopier.java:
##
@@ -157,9 +162,9 @@ public void join() throws InterruptedException {
public void cancel() {
canceled = true;
-LOG.info("Copier is canceled for partition [partId={}, tableId={}]",
partId(), tableId());
+LOG.info("Copier is canceled for partition [{}]",
createPartitionInfo());
-CompletableFuture fut = future;
+CompletableFuture fut = rebalanceFuture;
Review Comment:
Discussed personally, will work.
--
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 commented on a diff in pull request #1565: IGNITE-18086 Add index building in IncomingSnapshotCopier
rpuch commented on code in PR #1565:
URL: https://github.com/apache/ignite-3/pull/1565#discussion_r1084880005
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccessImpl.java:
##
@@ -213,4 +229,8 @@ private TxStateStorage getTxStateStorage(int partitionId) {
return txStateStorage;
}
+
+private void addToIndexes(TableRow tableRow, RowId rowId) {
+indexes.get().forEach(index -> index.put(tableRow, rowId));
Review Comment:
It looks like we forgot about deletions. For a deletion, on the sending
side, the `ReadResult.tableRow()` will return `null`, and
`ResponseEntry.rowVersions[i]' will be `null`, so `IncomingSnapshotCopier` on
line 268
```
TableRow tableRow = new TableRow(entry.rowVersions().get(i).rewind());
```
should make `tableRow = null` for a null `rowVersion` buffer, so
`addToIndexes()` will get a `null` `TableRow`, so it should check for null and
do not add anything to an index for a null value (actually, the code in
`PartitionListener` seems to do just this).
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccessImpl.java:
##
@@ -213,4 +229,8 @@ private TxStateStorage getTxStateStorage(int partitionId) {
return txStateStorage;
}
+
+private void addToIndexes(TableRow tableRow, RowId rowId) {
+indexes.get().forEach(index -> index.put(tableRow, rowId));
Review Comment:
Also, we probably need to add a scenario with a DELETE to our IT tests for
this feature. Existing tests only do INSERTs.
##
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccessImplTest.java:
##
@@ -119,4 +128,56 @@ void
testMinMaxLastAppliedTerm(@InjectConfiguration("mock.tables.foo {}") Tables
assertEquals(15, partitionAccess.minLastAppliedTerm());
assertEquals(20, partitionAccess.maxLastAppliedTerm());
}
+
+@Test
+void testAddWrite() {
+TestMvTableStorage mvTableStorage = new
TestMvTableStorage(tablesConfig.tables().get("foo"), tablesConfig);
+
+MvPartitionStorage mvPartitionStorage =
mvTableStorage.getOrCreateMvPartition(TEST_PARTITION_ID);
+
+TableSchemaAwareIndexStorage indexStorage =
mock(TableSchemaAwareIndexStorage.class);
+
+PartitionAccess partitionAccess = new PartitionAccessImpl(
+new PartitionKey(UUID.randomUUID(), TEST_PARTITION_ID),
+mvTableStorage,
+new TestTxStateTableStorage(),
+() -> List.of(indexStorage)
+);
+
+RowId rowId = new RowId(TEST_PARTITION_ID);
+TableRow tableRow = mock(TableRow.class);
+UUID txId = UUID.randomUUID();
+UUID commitTableId = UUID.randomUUID();
+
+partitionAccess.addWrite(rowId, tableRow, txId, commitTableId,
TEST_PARTITION_ID);
+
+verify(mvPartitionStorage, times(1)).addWrite(eq(rowId), eq(tableRow),
eq(txId), eq(commitTableId), eq(TEST_PARTITION_ID));
Review Comment:
Just a comment: `times(1)` is redundant, if we omit it, it will have the
same effect. But it can be used to highlight to the reader that you are
expecting exactly 1 call (not more), this can be useful sometimes.
--
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 #1562: IGNITE-18030 Integration of the new full rebalance API with IncomingSnapshotCopier
ibessonov commented on code in PR #1562:
URL: https://github.com/apache/ignite-3/pull/1562#discussion_r1084876619
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/incoming/IncomingSnapshotCopier.java:
##
@@ -96,38 +102,37 @@ public IncomingSnapshotCopier(PartitionSnapshotStorage
partitionSnapshotStorage,
public void start() {
Executor executor =
partitionSnapshotStorage.getIncomingSnapshotsExecutor();
-LOG.info("Copier is started for the partition [partId={},
tableId={}]", partId(), tableId());
+LOG.info("Copier is started for the partition [{}]",
createPartitionInfo());
+
+joinFuture = new CompletableFuture<>();
-future = prepareMvPartitionStorageForRebalance()
-.thenCompose(unused ->
prepareTxStatePartitionStorageForRebalance(executor))
+rebalanceFuture = partitionSnapshotStorage.partition().startRebalance()
.thenCompose(unused -> {
ClusterNode snapshotSender =
getSnapshotSender(snapshotUri.nodeName);
if (snapshotSender == null) {
-LOG.error(
-"Snapshot sender not found [partId={},
tableId={}, nodeName={}]",
-partId(),
-tableId(),
-snapshotUri.nodeName
-);
-
-if (!isOk()) {
-setError(RaftError.UNKNOWN, "Sender node was not
found or it is offline");
-}
-
-return completedFuture(null);
+throw new StorageRebalanceException("Snapshot sender
not found: " + snapshotUri.nodeName);
}
return loadSnapshotMeta(snapshotSender)
.thenCompose(unused1 ->
loadSnapshotMvData(snapshotSender, executor))
-.thenCompose(unused1 ->
loadSnapshotTxData(snapshotSender, executor))
-.thenAcceptAsync(unused1 ->
updateLastAppliedIndexFromSnapshotMetaForStorages(), executor);
+.thenCompose(unused1 ->
loadSnapshotTxData(snapshotSender, executor));
+});
+
+rebalanceFuture.handle((unused, throwable) ->
completesRebalance(throwable))
Review Comment:
Why "completes" instead of "complete"?
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/incoming/IncomingSnapshotCopier.java:
##
@@ -96,38 +102,37 @@ public IncomingSnapshotCopier(PartitionSnapshotStorage
partitionSnapshotStorage,
public void start() {
Executor executor =
partitionSnapshotStorage.getIncomingSnapshotsExecutor();
-LOG.info("Copier is started for the partition [partId={},
tableId={}]", partId(), tableId());
+LOG.info("Copier is started for the partition [{}]",
createPartitionInfo());
+
+joinFuture = new CompletableFuture<>();
-future = prepareMvPartitionStorageForRebalance()
-.thenCompose(unused ->
prepareTxStatePartitionStorageForRebalance(executor))
+rebalanceFuture = partitionSnapshotStorage.partition().startRebalance()
.thenCompose(unused -> {
ClusterNode snapshotSender =
getSnapshotSender(snapshotUri.nodeName);
if (snapshotSender == null) {
-LOG.error(
-"Snapshot sender not found [partId={},
tableId={}, nodeName={}]",
-partId(),
-tableId(),
-snapshotUri.nodeName
-);
-
-if (!isOk()) {
-setError(RaftError.UNKNOWN, "Sender node was not
found or it is offline");
-}
-
-return completedFuture(null);
+throw new StorageRebalanceException("Snapshot sender
not found: " + snapshotUri.nodeName);
}
return loadSnapshotMeta(snapshotSender)
.thenCompose(unused1 ->
loadSnapshotMvData(snapshotSender, executor))
-.thenCompose(unused1 ->
loadSnapshotTxData(snapshotSender, executor))
-.thenAcceptAsync(unused1 ->
updateLastAppliedIndexFromSnapshotMetaForStorages(), executor);
+.thenCompose(unused1 ->
loadSnapshotTxData(snapshotSender, executor));
+});
+
+rebalanceFuture.handle((unused, throwable) ->
completesRebalance(throwable))
+.thenCompose(Function.identity())
+.whenComplete((unused, throwable) -> {
+if (throwable == null) {
+joinFuture.complete(null);
Review
[GitHub] [ignite-3] zstan commented on a diff in pull request #1553: IGNITE-18580 Sql. Redesign the Exchange to use a pull-based approach
zstan commented on code in PR #1553:
URL: https://github.com/apache/ignite-3/pull/1553#discussion_r1083696938
##
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/ExchangeExecutionTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.sql.engine.exec.rel;
+
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.await;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectMaps;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.UUID;
+import java.util.function.Function;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.sql.engine.AsyncCursor.BatchedResult;
+import org.apache.ignite.internal.sql.engine.exec.ExchangeService;
+import org.apache.ignite.internal.sql.engine.exec.ExchangeServiceImpl;
+import org.apache.ignite.internal.sql.engine.exec.ExecutionContext;
+import org.apache.ignite.internal.sql.engine.exec.MailboxRegistry;
+import org.apache.ignite.internal.sql.engine.exec.MailboxRegistryImpl;
+import org.apache.ignite.internal.sql.engine.framework.DataProvider;
+import org.apache.ignite.internal.sql.engine.framework.TestBuilders;
+import org.apache.ignite.internal.sql.engine.message.MessageService;
+import org.apache.ignite.internal.sql.engine.message.MessageServiceImpl;
+import org.apache.ignite.internal.sql.engine.metadata.FragmentDescription;
+import org.apache.ignite.internal.sql.engine.trait.AllNodes;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.util.IgniteSpinBusyLock;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.network.ClusterService;
+import org.apache.ignite.network.NetworkAddress;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Tests to verify Outbox to Inbox interoperation.
+ */
+public class ExchangeExecutionTest extends AbstractExecutionTest {
+private static final String NODE_NAME = "N1";
+private static final ClusterNode LOCAL_NODE =
+new ClusterNode(NODE_NAME, NODE_NAME,
NetworkAddress.from("127.0.0.1:10001"));
+private static final int SOURCE_FRAGMENT_ID = 0;
+private static final int TARGET_FRAGMENT_ID = 1;
+
+@ParameterizedTest(name = "rowCount={0}, prefetch={1}, ordered={1}")
Review Comment:
```suggestion
@ParameterizedTest(name = "rowCount={0}, prefetch={1}, ordered={2}")
```
##
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/rel/ExchangeExecutionTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.sql.engine.exec.rel;
+
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.await;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectMaps;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.UUID;
+import java.util.function.Function;
+import java.util.stream.Stream;
+import org.apache.ignite.internal.sql.engine.AsyncCursor.BatchedResult;
+import org.apache.ignite.internal.sql.engine.exec.ExchangeService;
+import org.apache.ignite.internal.sql.engi
[GitHub] [ignite-3] zstan merged pull request #1550: IGNITE-18585 Sql. Introduce cache for serialized RelNodes representation
zstan merged PR #1550: URL: https://github.com/apache/ignite-3/pull/1550 -- 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-web-console] dependabot[bot] opened a new pull request, #23: Bump cookiejar from 2.1.2 to 2.1.4 in /modules/backend
dependabot[bot] opened a new pull request, #23: URL: https://github.com/apache/ignite-web-console/pull/23 Bumps [cookiejar](https://github.com/bmeck/node-cookiejar) from 2.1.2 to 2.1.4. Commits See full diff in https://github.com/bmeck/node-cookiejar/commits";>compare view [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/apache/ignite-web-console/network/alerts). -- 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] nizhikov opened a new pull request, #10492: IGNITE-18550 Incremental snapshot verify command
nizhikov opened a new pull request, #10492: URL: https://github.com/apache/ignite/pull/10492 Thank you for submitting the pull request to the Apache Ignite. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### The Contribution Checklist - [ ] There is a single JIRA ticket related to the pull request. - [ ] The web-link to the pull request is attached to the JIRA ticket. - [ ] The JIRA ticket has the _Patch Available_ state. - [ ] The pull request body describes changes that have been made. The description explains _WHAT_ and _WHY_ was made instead of _HOW_. - [ ] The pull request title is treated as the final commit message. The following pattern must be used: `IGNITE- Change summary` where `` - number of JIRA issue. - [ ] A reviewer has been mentioned through the JIRA comments (see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) - [ ] The pull request has been checked by the Teamcity Bot and the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html)) ### Notes - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute) - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules) - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines) - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot) If you need any help, please email d...@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel. -- 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, #1565: IGNITE-18086 Add index building in IncomingSnapshotCopier
tkalkirill opened a new pull request, #1565: URL: https://github.com/apache/ignite-3/pull/1565 https://issues.apache.org/jira/browse/IGNITE-18086 -- 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] vldpyatkov commented on a diff in pull request #1551: IGNITE-18527
vldpyatkov commented on code in PR #1551:
URL: https://github.com/apache/ignite-3/pull/1551#discussion_r1084200437
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##
@@ -1388,6 +1418,62 @@ private CompletableFuture
applyCmdWithExceptionHandling(Command cmd) {
});
}
+/**
+ * Executes an Update command.
+ *
+ * @param cmd Update command.
+ * @return Raft future, see {@link
#applyCmdWithExceptionHandling(Command)}.
+ */
+private CompletableFuture applyUpdateCommand(UpdateCommand cmd) {
Review Comment:
Honestly, I do not see a sense to divide applyCmdWithExceptionHandling and
updateCommand in two difference method, because they always execute both. In
your case. I would merge applyUpdateCommand and updateCommand.
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##
@@ -1388,6 +1418,62 @@ private CompletableFuture
applyCmdWithExceptionHandling(Command cmd) {
});
}
+/**
+ * Executes an Update command.
+ *
+ * @param cmd Update command.
+ * @return Raft future, see {@link
#applyCmdWithExceptionHandling(Command)}.
+ */
+private CompletableFuture applyUpdateCommand(UpdateCommand cmd) {
+return applyUpdatingCommand(cmd.txId(), () -> {
+storageUpdateHandler.handleUpdate(
+cmd.txId(),
+cmd.rowUuid(),
+cmd.tablePartitionId().asTablePartitionId(),
+cmd.rowBuffer(),
+null
+);
+
+return applyCmdWithExceptionHandling(cmd);
+});
+}
+
+/**
+ * Executes an UpdateAll command.
+ *
+ * @param cmd UpdateAll command.
+ * @return Raft future, see {@link
#applyCmdWithExceptionHandling(Command)}.
+ */
+private CompletableFuture applyUpdateAllCommand(UpdateAllCommand
cmd) {
Review Comment:
The same as before, I wouldn't divide applyUpdateAllCommand and
updateAllCommand.
--
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 #1561: IGNITE-18600 Save Meta Storage watch data to the Vault
sanpwc merged PR #1561: URL: https://github.com/apache/ignite-3/pull/1561 -- 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 #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083916784
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccess.java:
##
@@ -32,27 +41,111 @@ public interface PartitionAccess {
PartitionKey partitionKey();
/**
- * Returns the multi-versioned partition storage.
+ * Destroys and recreates the multi-versioned partition storage.
+ *
+ * @return Future that will complete when the partition is recreated.
+ * @throws StorageException If an error has occurred during the partition
destruction.
*/
-MvPartitionStorage mvPartitionStorage();
+CompletableFuture reCreateMvPartitionStorage() throws
StorageException;
/**
- * Returns transaction state storage for the partition.
+ * Destroys and recreates the multi-versioned partition storage.
+ *
+ * @throws StorageException If an error has occurred during transaction
state storage for the partition destruction.
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] meanother commented on issue #10491: CorruptedTreeException: B+Tree is corrupted
meanother commented on issue #10491: URL: https://github.com/apache/ignite/issues/10491#issuecomment-1400429078 1) Create table `X`, fill it with data (or they are already there) 2) Create table saa_sv_rawdbo_dm_transactions_month, create index idx_fin_client (both entities are empty) 3) We fill the table saa_sv_rawdbo_dm_transactions_month with data - via ``` insert into saa_sv_rawdbo_dm_transactions_month select ... from X where condition ``` 4) we fall with an error -- 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 commented on issue #10491: CorruptedTreeException: B+Tree is corrupted
ivandasch commented on issue #10491: URL: https://github.com/apache/ignite/issues/10491#issuecomment-1400415252 @meanother Do I understand correctly, that you have tried to create an additional index on the key field and this action led to a failure? -- 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] meanother opened a new issue, #10491: CorruptedTreeException: B+Tree is corrupted
meanother opened a new issue, #10491: URL: https://github.com/apache/ignite/issues/10491 Java version 1.8 Ignite version 2.14 ``` CREATE TABLE saa_sv_rawdbo_dm_transactions_month( pay_client_pin VARCHAR , pay_transaction_count int , pay_tarif_plan_set VARCHAR ... , PRIMARY KEY (pay_client_pin) ) WITH "affinityKey=pay_client_pin,DATA_REGION=persist_data_region,template=onheapcache"; ``` `CREATE INDEX idx_fin_client ON public.saa_sv_rawdbo_dm_transactions_month(pay_client_pin) INLINE_SIZE 6;` ``` ``` log: ``` [12:38:31,424][SEVERE][build-idx-runner-#228][] Critical system error detected. Will be handled accordingly to configured handler [hnd=StopNodeOrHaltFailureHandler [tryStop=false, timeout=0, super=AbstractFailureHandler [ignoredFailureTypes=UnmodifiableSet [SYSTEM_WORKER_BLOCKED, SYSTEM_CRITICAL_OPERATION_TIMEOUT]]], failureCtx=FailureContext [type=CRITICAL_ERROR, err=class o.a.i.i.processors.cache.persistence.tree.CorruptedTreeException: B+Tree is corrupted [groupId=-288979615, pageIds=[844420635189922], cacheId=-288979615, cacheName=SQL_PUBLIC_SAA_SV_RAWDBO_DM_TRANSACTIONS_MONTH, indexName=IDX_FIN_CLIENT, msg=Runtime failure on row: Row@5c1a42b0[ key: A201IX, val: SQL_PUBLIC_SAA_SV_RAWDBO_DM_TRANSACTIONS_MONTH_177aaa96_e8b5_4ee1_baff_10353741c660 [idHash=1390257987, hash=-511005807, PAY_TRANSACTION_COUNT=2, ] ][ A201IX, A201IX class org.apache.ignite.internal.processors.cache.persistence.tree.CorruptedTreeException: B+Tree is corrupted [groupId=-288979615, pageIds=[844420635189922], cacheId=-288979615, cacheName=SQL_PUBLIC_SAA_SV_RAWDBO_DM_TRANSACTIONS_MONTH, indexName=IDX_FIN_CLIENT, msg=Runtime failure on row: Row@5c1a42b0[ key: A201IX, val: SQL_PUBLIC_SAA_SV_RAWDBO_DM_TRANSACTIONS_MONTH_177aaa96_e8b5_4ee1_baff_10353741c660 [idHash=1390257987, hash=-511005807, PAY_TRANSACTION_COUNT=2, PAY_TARIF_PLAN_SET=11] ][ A201IX, A201IX ]] at org.apache.ignite.internal.cache.query.index.sorted.inline.InlineIndexTree.corruptedTreeException(InlineIndexTree.java:561) at org.apache.ignite.internal.processors.cache.persistence.tree.BPlusTree.doPut(BPlusTree.java:2671) at org.apache.ignite.internal.processors.cache.persistence.tree.BPlusTree.put(BPlusTree.java:2602) at org.apache.ignite.internal.cache.query.index.sorted.inline.InlineIndexImpl.putx(InlineIndexImpl.java:348) at org.apache.ignite.internal.cache.query.index.sorted.inline.InlineIndexImpl.onUpdate(InlineIndexImpl.java:325) at org.apache.ignite.internal.cache.query.index.IndexProcessor.lambda$createIndexDynamically$0(IndexProcessor.java:225) at org.apache.ignite.internal.processors.query.schema.SchemaIndexCachePartitionWorker$SchemaIndexCacheVisitorClosureWrapper.apply(SchemaIndexCachePartitionWorker.java:302) at org.apache.ignite.internal.processors.cache.GridCacheMapEntry.updateIndex(GridCacheMapEntry.java:4193) at org.apache.ignite.internal.processors.query.schema.SchemaIndexCachePartitionWorker.processKey(SchemaIndexCachePartitionWorker.java:236) at org.apache.ignite.internal.processors.query.schema.SchemaIndexCachePartitionWorker.processPartition(SchemaIndexCachePartitionWorker.java:191) at org.apache.ignite.internal.processors.query.schema.SchemaIndexCachePartitionWorker.body(SchemaIndexCachePartitionWorker.java:130) at org.apache.ignite.internal.util.worker.GridWorker.run(GridWorker.java:125) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) at java.lang.Thread.run(Thread.java:750) Caused by: class org.apache.ignite.IgniteException: Failed to store new index row. at org.apache.ignite.internal.cache.query.index.sorted.inline.io.AbstractInlineInnerIO.storeByOffset(AbstractInlineInnerIO.java:106) at org.apache.ignite.internal.cache.query.index.sorted.inline.io.AbstractInlineInnerIO.storeByOffset(AbstractInlineInnerIO.java:37) at org.apache.ignite.internal.processors.cache.persistence.tree.io.BPlusIO.store(BPlusIO.java:228) at org.apache.ignite.internal.processors.cache.persistence.tree.io.BPlusIO.insert(BPlusIO.java:317) at org.apache.ignite.internal.processors.cache.persistence.tree.io.BPlusInnerIO.insert(BPlusInnerIO.java:150) at org.apache.ignite.internal.processors.cache.persistence.tree.BPlusTree$Put.insertSimple(BPlusTree.java:3962) at org.apache.ignite.internal.processors.cache.persistence.tree.BPlusTree$Put.insert(BPlusTree.java:3944) at org.apache.ignite.internal.processors.cache.persistence.tree.BPlusTree
[GitHub] [ignite] nizhikov merged pull request #10485: IGNITE-18232 Remove of CacheMetricsMXBean legacy JMX bean
nizhikov merged PR #10485: URL: https://github.com/apache/ignite/pull/10485 -- 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 merged pull request #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill merged PR #1556: URL: https://github.com/apache/ignite-3/pull/1556 -- 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] timoninmaxim merged pull request #10353: IGNITE-17648 Extract CacheStripedExecutor to separate class
timoninmaxim merged PR #10353: URL: https://github.com/apache/ignite/pull/10353 -- 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] alex-plekhanov commented on a diff in pull request #10479: IGNITE-18511 Added details to Calcite parser exception
alex-plekhanov commented on code in PR #10479:
URL: https://github.com/apache/ignite/pull/10479#discussion_r1083965015
##
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDdlIntegrationTest.java:
##
@@ -281,6 +281,24 @@ public void createTableIfNotExists() {
sql("create table if not exists my_table (id int, val varchar)");
}
+/**
+ * Create table using reserved word
+ */
+@Test
+public void createTableUseReservedWord() {
+assertThrows("create table table (id int primary key, val varchar)",
IgniteSQLException.class,
+"Was expecting one of:");
+
+sql("create table \"table\" (id int primary key, val varchar)");
+
+sql("insert into \"table\" (id, val) values (0, '1')");
+
+List> res = sql("select * from \"table\" ");
+
+assertEquals(1, res.size());
+assertEquals(2, res.get(0).size());
Review Comment:
`assertQuery("select * from \"table\" ").returns(0, "1").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] tkalkirill commented on a diff in pull request #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083946905
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccessImpl.java:
##
@@ -57,48 +70,147 @@ public PartitionKey partitionKey() {
}
@Override
-public MvPartitionStorage mvPartitionStorage() {
-MvPartitionStorage mvPartition =
mvTableStorage.getMvPartition(partId());
+public CompletableFuture reCreateMvPartitionStorage() throws
StorageException {
+assert mvTableStorage.getMvPartition(partitionId()) != null : "table="
+ tableName() + ", part=" + partitionId();
+
+// TODO: IGNITE-18030 - actually recreate or do in a different way
+//return mvTableStorage.destroyPartition(partId())
+return CompletableFuture.completedFuture(null)
+.thenApply(unused -> {
+MvPartitionStorage mvPartitionStorage =
mvTableStorage.getOrCreateMvPartition(partitionId());
+
+mvPartitionStorage.runConsistently(() -> {
+
mvPartitionStorage.lastApplied(FULL_RABALANCING_STARTED, 0);
-assert mvPartition != null : "table=" + tableName() + ", part=" +
partId();
+return null;
+});
-return mvPartition;
+return null;
+});
}
@Override
-public TxStateStorage txStatePartitionStorage() {
-TxStateStorage txStatePartitionStorage =
txStateTableStorage.getTxStateStorage(partId());
+public void reCreateTxStatePartitionStorage() throws StorageException {
+assert txStateTableStorage.getTxStateStorage(partitionId()) != null :
"table=" + tableName() + ", part=" + partitionId();
+
+// TODO: IGNITE-18030 - actually recreate or do in a different way
+//txStateTableStorage.destroyTxStateStorage(partId());
+
+
txStateTableStorage.getOrCreateTxStateStorage(partitionId()).lastApplied(FULL_RABALANCING_STARTED,
0);
+}
-assert txStatePartitionStorage != null : "table=" + tableName() + ",
part=" + partId();
+private int partitionId() {
+return partitionKey.partitionId();
+}
-return txStatePartitionStorage;
+private String tableName() {
+return mvTableStorage.configuration().name().value();
}
@Override
-public CompletableFuture reCreateMvPartitionStorage()
throws StorageException {
-assert mvTableStorage.getMvPartition(partId()) != null : "table=" +
tableName() + ", part=" + partId();
+public Cursor> getAllTxMeta() {
+return getTxStateStorage(partitionId()).scan();
+}
-// TODO: IGNITE-18030 - actually recreate or do in a different way
-//return mvTableStorage.destroyPartition(partId())
-return CompletableFuture.completedFuture(null)
-.thenApply(unused ->
mvTableStorage.getOrCreateMvPartition(partId()));
+@Override
+public void addTxMeta(UUID txId, TxMeta txMeta) {
+getTxStateStorage(partitionId()).put(txId, txMeta);
}
@Override
-public TxStateStorage reCreateTxStatePartitionStorage() throws
StorageException {
-assert txStateTableStorage.getTxStateStorage(partId()) != null :
"table=" + tableName() + ", part=" + partId();
+public @Nullable RowId closestRowId(RowId lowerBound) {
+return getMvPartitionStorage(partitionId()).closestRowId(lowerBound);
+}
-// TODO: IGNITE-18030 - actually recreate or do in a different way
-//txStateTableStorage.destroyTxStateStorage(partId());
+@Override
+public Cursor getAllRowVersions(RowId rowId) {
+return getMvPartitionStorage(partitionId()).scanVersions(rowId);
+}
-return txStateTableStorage.getOrCreateTxStateStorage(partId());
+@Override
+public @Nullable RaftGroupConfiguration committedGroupConfiguration() {
+return
getMvPartitionStorage(partitionId()).committedGroupConfiguration();
}
-private int partId() {
-return partitionKey.partitionId();
+@Override
+public void addWrite(RowId rowId, TableRow row, UUID txId, UUID
commitTableId, int commitPartitionId) {
+MvPartitionStorage mvPartitionStorage =
getMvPartitionStorage(partitionId());
+
+mvPartitionStorage.runConsistently(() ->
mvPartitionStorage.addWrite(rowId, row, txId, commitTableId,
commitPartitionId));
}
-private String tableName() {
-return mvTableStorage.configuration().name().value();
+@Override
+public void addWriteCommitted(RowId rowId, TableRow row, HybridTimestamp
commitTimestamp) {
+MvPartitionStorage mvPartitionStorage =
getMvPartitionStorage(partitionId());
+
+mvPartitionStorage.runConsistently(() -> {
+mvPartitionStorage.addWriteCommitted(rowId, row, commitTimestamp);
+
+return null;
+});
+}
+
+@Override
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10485: IGNITE-18232 : [IEP-80] Remove of CacheMetricsMXBean legacy JMX bean, v3 : keep JCache beans
Vladsz83 commented on code in PR #10485:
URL: https://github.com/apache/ignite/pull/10485#discussion_r1083958072
##
modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/RestMemcacheProtocolSelfTest.java:
##
@@ -185,8 +185,8 @@ public void testReplace() throws Exception {
*/
@Test
public void testMetrics() throws Exception {
-grid().cache(DEFAULT_CACHE_NAME).localMxBean().clear();
-grid().cache(CACHE_NAME).localMxBean().clear();
+grid().cache(DEFAULT_CACHE_NAME).clearStatistics();
Review Comment:
> > You always could clear cache statistics by calling Ignite cache API
>
> What API, exactly? Why in tests you are using private API?
It is not private. It uses IgniteCache#clearStatistics(). It is a public API.
--
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] nizhikov commented on a diff in pull request #10485: IGNITE-18232 : [IEP-80] Remove of CacheMetricsMXBean legacy JMX bean, v3 : keep JCache beans
nizhikov commented on code in PR #10485:
URL: https://github.com/apache/ignite/pull/10485#discussion_r1083956343
##
modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/RestMemcacheProtocolSelfTest.java:
##
@@ -185,8 +185,8 @@ public void testReplace() throws Exception {
*/
@Test
public void testMetrics() throws Exception {
-grid().cache(DEFAULT_CACHE_NAME).localMxBean().clear();
-grid().cache(CACHE_NAME).localMxBean().clear();
+grid().cache(DEFAULT_CACHE_NAME).clearStatistics();
Review Comment:
> You always could clear cache statistics by calling Ignite cache API
What API, exactly? Why in tests you are using private API?
--
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 commented on a diff in pull request #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
rpuch commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083937026
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccessImpl.java:
##
@@ -57,48 +70,147 @@ public PartitionKey partitionKey() {
}
@Override
-public MvPartitionStorage mvPartitionStorage() {
-MvPartitionStorage mvPartition =
mvTableStorage.getMvPartition(partId());
+public CompletableFuture reCreateMvPartitionStorage() throws
StorageException {
+assert mvTableStorage.getMvPartition(partitionId()) != null : "table="
+ tableName() + ", part=" + partitionId();
+
+// TODO: IGNITE-18030 - actually recreate or do in a different way
+//return mvTableStorage.destroyPartition(partId())
+return CompletableFuture.completedFuture(null)
+.thenApply(unused -> {
+MvPartitionStorage mvPartitionStorage =
mvTableStorage.getOrCreateMvPartition(partitionId());
+
+mvPartitionStorage.runConsistently(() -> {
+
mvPartitionStorage.lastApplied(FULL_RABALANCING_STARTED, 0);
-assert mvPartition != null : "table=" + tableName() + ", part=" +
partId();
+return null;
+});
-return mvPartition;
+return null;
+});
}
@Override
-public TxStateStorage txStatePartitionStorage() {
-TxStateStorage txStatePartitionStorage =
txStateTableStorage.getTxStateStorage(partId());
+public void reCreateTxStatePartitionStorage() throws StorageException {
+assert txStateTableStorage.getTxStateStorage(partitionId()) != null :
"table=" + tableName() + ", part=" + partitionId();
+
+// TODO: IGNITE-18030 - actually recreate or do in a different way
+//txStateTableStorage.destroyTxStateStorage(partId());
+
+
txStateTableStorage.getOrCreateTxStateStorage(partitionId()).lastApplied(FULL_RABALANCING_STARTED,
0);
+}
-assert txStatePartitionStorage != null : "table=" + tableName() + ",
part=" + partId();
+private int partitionId() {
+return partitionKey.partitionId();
+}
-return txStatePartitionStorage;
+private String tableName() {
+return mvTableStorage.configuration().name().value();
}
@Override
-public CompletableFuture reCreateMvPartitionStorage()
throws StorageException {
-assert mvTableStorage.getMvPartition(partId()) != null : "table=" +
tableName() + ", part=" + partId();
+public Cursor> getAllTxMeta() {
+return getTxStateStorage(partitionId()).scan();
+}
-// TODO: IGNITE-18030 - actually recreate or do in a different way
-//return mvTableStorage.destroyPartition(partId())
-return CompletableFuture.completedFuture(null)
-.thenApply(unused ->
mvTableStorage.getOrCreateMvPartition(partId()));
+@Override
+public void addTxMeta(UUID txId, TxMeta txMeta) {
+getTxStateStorage(partitionId()).put(txId, txMeta);
}
@Override
-public TxStateStorage reCreateTxStatePartitionStorage() throws
StorageException {
-assert txStateTableStorage.getTxStateStorage(partId()) != null :
"table=" + tableName() + ", part=" + partId();
+public @Nullable RowId closestRowId(RowId lowerBound) {
+return getMvPartitionStorage(partitionId()).closestRowId(lowerBound);
+}
-// TODO: IGNITE-18030 - actually recreate or do in a different way
-//txStateTableStorage.destroyTxStateStorage(partId());
+@Override
+public Cursor getAllRowVersions(RowId rowId) {
+return getMvPartitionStorage(partitionId()).scanVersions(rowId);
+}
-return txStateTableStorage.getOrCreateTxStateStorage(partId());
+@Override
+public @Nullable RaftGroupConfiguration committedGroupConfiguration() {
+return
getMvPartitionStorage(partitionId()).committedGroupConfiguration();
}
-private int partId() {
-return partitionKey.partitionId();
+@Override
+public void addWrite(RowId rowId, TableRow row, UUID txId, UUID
commitTableId, int commitPartitionId) {
+MvPartitionStorage mvPartitionStorage =
getMvPartitionStorage(partitionId());
+
+mvPartitionStorage.runConsistently(() ->
mvPartitionStorage.addWrite(rowId, row, txId, commitTableId,
commitPartitionId));
}
-private String tableName() {
-return mvTableStorage.configuration().name().value();
+@Override
+public void addWriteCommitted(RowId rowId, TableRow row, HybridTimestamp
commitTimestamp) {
+MvPartitionStorage mvPartitionStorage =
getMvPartitionStorage(partitionId());
+
+mvPartitionStorage.runConsistently(() -> {
+mvPartitionStorage.addWriteCommitted(rowId, row, commitTimestamp);
+
+return null;
+});
+}
+
+@Override
+
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10485: IGNITE-18232 : [IEP-80] Remove of CacheMetricsMXBean legacy JMX bean, v3 : keep JCache beans
Vladsz83 commented on code in PR #10485:
URL: https://github.com/apache/ignite/pull/10485#discussion_r1083943685
##
modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/RestMemcacheProtocolSelfTest.java:
##
@@ -185,8 +185,8 @@ public void testReplace() throws Exception {
*/
@Test
public void testMetrics() throws Exception {
-grid().cache(DEFAULT_CACHE_NAME).localMxBean().clear();
-grid().cache(CACHE_NAME).localMxBean().clear();
+grid().cache(DEFAULT_CACHE_NAME).clearStatistics();
Review Comment:
> > This method calls GridCacheProcessor#clearStatistics().
>
> How to do the same with public API?
You always could clear cache statistics by calling Ignite cache API and by
calling mx bean. Regarding to the IEP, we assume mx bean API is obsolete. Why
using it again? Consider we wouldn't have to keep a mxbean to support any spec
like JCache spec. There would be no mxbean alt all and no old mxbean#clear()
method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10485: IGNITE-18232 : [IEP-80] Remove of CacheMetricsMXBean legacy JMX bean, v3 : keep JCache beans
Vladsz83 commented on code in PR #10485:
URL: https://github.com/apache/ignite/pull/10485#discussion_r1083943685
##
modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/RestMemcacheProtocolSelfTest.java:
##
@@ -185,8 +185,8 @@ public void testReplace() throws Exception {
*/
@Test
public void testMetrics() throws Exception {
-grid().cache(DEFAULT_CACHE_NAME).localMxBean().clear();
-grid().cache(CACHE_NAME).localMxBean().clear();
+grid().cache(DEFAULT_CACHE_NAME).clearStatistics();
Review Comment:
> > This method calls GridCacheProcessor#clearStatistics().
>
> How to do the same with public API?
You always could clear cache statistics by calling Ignite cache API and by
calling mx bean. Regarding to the IEP, we assume mx bean api pbsolete. Why
using It again? Consider we wouldn't have to keep mxbean to support any spec
like JCache spec. There would be no mxbean and no old mxbean#clear() method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: 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 #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083941400
##
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshotCommonTest.java:
##
@@ -76,12 +64,13 @@ void returnsKeyFromStorage() {
@Test
void sendsSnapshotMeta() {
-when(mvPartitionStorage.lastAppliedIndex()).thenReturn(100L);
-lenient().when(mvPartitionStorage.lastAppliedTerm()).thenReturn(3L);
-when(txStateStorage.lastAppliedIndex()).thenReturn(100L);
-lenient().when(txStateStorage.lastAppliedTerm()).thenReturn(3L);
+lenient().when(partitionAccess.minLastAppliedIndex()).thenReturn(90L);
Review Comment:
Fix it
##
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionSnapshotStorageFactoryTest.java:
##
@@ -44,24 +39,18 @@
*/
@ExtendWith(MockitoExtension.class)
public class PartitionSnapshotStorageFactoryTest {
-private final MvPartitionStorage mvPartitionStorage = new
TestMvPartitionStorage(0);
-private final TxStateStorage txStateStorage = new TestTxStateStorage();
-
@Mock
private PartitionAccess partitionAccess;
-@BeforeEach
-void configureMocks() {
-
when(partitionAccess.mvPartitionStorage()).thenReturn(mvPartitionStorage);
-
when(partitionAccess.txStatePartitionStorage()).thenReturn(txStateStorage);
-}
-
@Test
void testForChoosingMinimumAppliedIndexForMeta() {
-mvPartitionStorage.lastApplied(10L, 2L);
-txStateStorage.lastApplied(5L, 1L);
+lenient().when(partitionAccess.minLastAppliedIndex()).thenReturn(5L);
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] vldpyatkov opened a new pull request, #1563: IGNITE-18358 IGN-TX-5 on concurrent transactional single key load
vldpyatkov opened a new pull request, #1563: URL: https://github.com/apache/ignite-3/pull/1563 https://issues.apache.org/jira/browse/IGNITE-18358 -- 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_r1083927134
##
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:
let's add _execution_ test for this case
--
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 #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083924403
##
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionSnapshotStorageFactoryTest.java:
##
@@ -44,24 +39,18 @@
*/
@ExtendWith(MockitoExtension.class)
public class PartitionSnapshotStorageFactoryTest {
-private final MvPartitionStorage mvPartitionStorage = new
TestMvPartitionStorage(0);
-private final TxStateStorage txStateStorage = new TestTxStateStorage();
-
@Mock
private PartitionAccess partitionAccess;
-@BeforeEach
-void configureMocks() {
-
when(partitionAccess.mvPartitionStorage()).thenReturn(mvPartitionStorage);
-
when(partitionAccess.txStatePartitionStorage()).thenReturn(txStateStorage);
-}
-
@Test
void testForChoosingMinimumAppliedIndexForMeta() {
-mvPartitionStorage.lastApplied(10L, 2L);
-txStateStorage.lastApplied(5L, 1L);
+lenient().when(partitionAccess.minLastAppliedIndex()).thenReturn(5L);
Review Comment:
I tried to delete it, but got the following error: `Please remove
unnecessary stubbings or use 'lenient' strictness. More info: javadoc for
UnnecessaryStubbingException 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] rpuch commented on a diff in pull request #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
rpuch commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083932748
##
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshotCommonTest.java:
##
@@ -76,12 +64,13 @@ void returnsKeyFromStorage() {
@Test
void sendsSnapshotMeta() {
-when(mvPartitionStorage.lastAppliedIndex()).thenReturn(100L);
-lenient().when(mvPartitionStorage.lastAppliedTerm()).thenReturn(3L);
-when(txStateStorage.lastAppliedIndex()).thenReturn(100L);
-lenient().when(txStateStorage.lastAppliedTerm()).thenReturn(3L);
+lenient().when(partitionAccess.minLastAppliedIndex()).thenReturn(90L);
Review Comment:
Then how about removing the whole line? If Mockito says the stubbing is not
used, then we probably don't need it at all, right?
--
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 #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083926252
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccessImpl.java:
##
@@ -57,48 +70,147 @@ public PartitionKey partitionKey() {
}
@Override
-public MvPartitionStorage mvPartitionStorage() {
-MvPartitionStorage mvPartition =
mvTableStorage.getMvPartition(partId());
+public CompletableFuture reCreateMvPartitionStorage() throws
StorageException {
+assert mvTableStorage.getMvPartition(partitionId()) != null : "table="
+ tableName() + ", part=" + partitionId();
+
+// TODO: IGNITE-18030 - actually recreate or do in a different way
+//return mvTableStorage.destroyPartition(partId())
+return CompletableFuture.completedFuture(null)
+.thenApply(unused -> {
+MvPartitionStorage mvPartitionStorage =
mvTableStorage.getOrCreateMvPartition(partitionId());
+
+mvPartitionStorage.runConsistently(() -> {
+
mvPartitionStorage.lastApplied(FULL_RABALANCING_STARTED, 0);
-assert mvPartition != null : "table=" + tableName() + ", part=" +
partId();
+return null;
+});
-return mvPartition;
+return null;
+});
}
@Override
-public TxStateStorage txStatePartitionStorage() {
-TxStateStorage txStatePartitionStorage =
txStateTableStorage.getTxStateStorage(partId());
+public void reCreateTxStatePartitionStorage() throws StorageException {
+assert txStateTableStorage.getTxStateStorage(partitionId()) != null :
"table=" + tableName() + ", part=" + partitionId();
+
+// TODO: IGNITE-18030 - actually recreate or do in a different way
+//txStateTableStorage.destroyTxStateStorage(partId());
+
+
txStateTableStorage.getOrCreateTxStateStorage(partitionId()).lastApplied(FULL_RABALANCING_STARTED,
0);
+}
-assert txStatePartitionStorage != null : "table=" + tableName() + ",
part=" + partId();
+private int partitionId() {
+return partitionKey.partitionId();
+}
-return txStatePartitionStorage;
+private String tableName() {
+return mvTableStorage.configuration().name().value();
}
@Override
-public CompletableFuture reCreateMvPartitionStorage()
throws StorageException {
-assert mvTableStorage.getMvPartition(partId()) != null : "table=" +
tableName() + ", part=" + partId();
+public Cursor> getAllTxMeta() {
+return getTxStateStorage(partitionId()).scan();
+}
-// TODO: IGNITE-18030 - actually recreate or do in a different way
-//return mvTableStorage.destroyPartition(partId())
-return CompletableFuture.completedFuture(null)
-.thenApply(unused ->
mvTableStorage.getOrCreateMvPartition(partId()));
+@Override
+public void addTxMeta(UUID txId, TxMeta txMeta) {
+getTxStateStorage(partitionId()).put(txId, txMeta);
}
@Override
-public TxStateStorage reCreateTxStatePartitionStorage() throws
StorageException {
-assert txStateTableStorage.getTxStateStorage(partId()) != null :
"table=" + tableName() + ", part=" + partId();
+public @Nullable RowId closestRowId(RowId lowerBound) {
+return getMvPartitionStorage(partitionId()).closestRowId(lowerBound);
+}
-// TODO: IGNITE-18030 - actually recreate or do in a different way
-//txStateTableStorage.destroyTxStateStorage(partId());
+@Override
+public Cursor getAllRowVersions(RowId rowId) {
+return getMvPartitionStorage(partitionId()).scanVersions(rowId);
+}
-return txStateTableStorage.getOrCreateTxStateStorage(partId());
+@Override
+public @Nullable RaftGroupConfiguration committedGroupConfiguration() {
+return
getMvPartitionStorage(partitionId()).committedGroupConfiguration();
}
-private int partId() {
-return partitionKey.partitionId();
+@Override
+public void addWrite(RowId rowId, TableRow row, UUID txId, UUID
commitTableId, int commitPartitionId) {
+MvPartitionStorage mvPartitionStorage =
getMvPartitionStorage(partitionId());
+
+mvPartitionStorage.runConsistently(() ->
mvPartitionStorage.addWrite(rowId, row, txId, commitTableId,
commitPartitionId));
}
-private String tableName() {
-return mvTableStorage.configuration().name().value();
+@Override
+public void addWriteCommitted(RowId rowId, TableRow row, HybridTimestamp
commitTimestamp) {
+MvPartitionStorage mvPartitionStorage =
getMvPartitionStorage(partitionId());
+
+mvPartitionStorage.runConsistently(() -> {
+mvPartitionStorage.addWriteCommitted(rowId, row, commitTimestamp);
+
+return null;
+});
+}
+
+@Override
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083926252
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccessImpl.java:
##
@@ -57,48 +70,147 @@ public PartitionKey partitionKey() {
}
@Override
-public MvPartitionStorage mvPartitionStorage() {
-MvPartitionStorage mvPartition =
mvTableStorage.getMvPartition(partId());
+public CompletableFuture reCreateMvPartitionStorage() throws
StorageException {
+assert mvTableStorage.getMvPartition(partitionId()) != null : "table="
+ tableName() + ", part=" + partitionId();
+
+// TODO: IGNITE-18030 - actually recreate or do in a different way
+//return mvTableStorage.destroyPartition(partId())
+return CompletableFuture.completedFuture(null)
+.thenApply(unused -> {
+MvPartitionStorage mvPartitionStorage =
mvTableStorage.getOrCreateMvPartition(partitionId());
+
+mvPartitionStorage.runConsistently(() -> {
+
mvPartitionStorage.lastApplied(FULL_RABALANCING_STARTED, 0);
-assert mvPartition != null : "table=" + tableName() + ", part=" +
partId();
+return null;
+});
-return mvPartition;
+return null;
+});
}
@Override
-public TxStateStorage txStatePartitionStorage() {
-TxStateStorage txStatePartitionStorage =
txStateTableStorage.getTxStateStorage(partId());
+public void reCreateTxStatePartitionStorage() throws StorageException {
+assert txStateTableStorage.getTxStateStorage(partitionId()) != null :
"table=" + tableName() + ", part=" + partitionId();
+
+// TODO: IGNITE-18030 - actually recreate or do in a different way
+//txStateTableStorage.destroyTxStateStorage(partId());
+
+
txStateTableStorage.getOrCreateTxStateStorage(partitionId()).lastApplied(FULL_RABALANCING_STARTED,
0);
+}
-assert txStatePartitionStorage != null : "table=" + tableName() + ",
part=" + partId();
+private int partitionId() {
+return partitionKey.partitionId();
+}
-return txStatePartitionStorage;
+private String tableName() {
+return mvTableStorage.configuration().name().value();
}
@Override
-public CompletableFuture reCreateMvPartitionStorage()
throws StorageException {
-assert mvTableStorage.getMvPartition(partId()) != null : "table=" +
tableName() + ", part=" + partId();
+public Cursor> getAllTxMeta() {
+return getTxStateStorage(partitionId()).scan();
+}
-// TODO: IGNITE-18030 - actually recreate or do in a different way
-//return mvTableStorage.destroyPartition(partId())
-return CompletableFuture.completedFuture(null)
-.thenApply(unused ->
mvTableStorage.getOrCreateMvPartition(partId()));
+@Override
+public void addTxMeta(UUID txId, TxMeta txMeta) {
+getTxStateStorage(partitionId()).put(txId, txMeta);
}
@Override
-public TxStateStorage reCreateTxStatePartitionStorage() throws
StorageException {
-assert txStateTableStorage.getTxStateStorage(partId()) != null :
"table=" + tableName() + ", part=" + partId();
+public @Nullable RowId closestRowId(RowId lowerBound) {
+return getMvPartitionStorage(partitionId()).closestRowId(lowerBound);
+}
-// TODO: IGNITE-18030 - actually recreate or do in a different way
-//txStateTableStorage.destroyTxStateStorage(partId());
+@Override
+public Cursor getAllRowVersions(RowId rowId) {
+return getMvPartitionStorage(partitionId()).scanVersions(rowId);
+}
-return txStateTableStorage.getOrCreateTxStateStorage(partId());
+@Override
+public @Nullable RaftGroupConfiguration committedGroupConfiguration() {
+return
getMvPartitionStorage(partitionId()).committedGroupConfiguration();
}
-private int partId() {
-return partitionKey.partitionId();
+@Override
+public void addWrite(RowId rowId, TableRow row, UUID txId, UUID
commitTableId, int commitPartitionId) {
+MvPartitionStorage mvPartitionStorage =
getMvPartitionStorage(partitionId());
+
+mvPartitionStorage.runConsistently(() ->
mvPartitionStorage.addWrite(rowId, row, txId, commitTableId,
commitPartitionId));
}
-private String tableName() {
-return mvTableStorage.configuration().name().value();
+@Override
+public void addWriteCommitted(RowId rowId, TableRow row, HybridTimestamp
commitTimestamp) {
+MvPartitionStorage mvPartitionStorage =
getMvPartitionStorage(partitionId());
+
+mvPartitionStorage.runConsistently(() -> {
+mvPartitionStorage.addWriteCommitted(rowId, row, commitTimestamp);
+
+return null;
+});
+}
+
+@Override
[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_r1083924778
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSortAggregateTest.java:
##
@@ -67,8 +70,134 @@ public void mapReduceAggregate() {
@Test
public void correctCollationsOnMapReduceSortAgg() {
-var cursors = sql("SELECT PK FROM TEST_ONE_COL_IDX WHERE col0 IN
(SELECT col0 FROM TEST_ONE_COL_IDX)");
+String disabledRules = " /*+
DISABLE_RULE('MapReduceHashAggregateConverterRule',
'ColocatedSortAggregateConverterRule') */ ";
+
+var cursors = sql(
+appendDisabledRules("SELECT PK FROM TEST_ONE_COL_IDX WHERE
col0 IN (SELECT col0 FROM TEST_ONE_COL_IDX)", disabledRules));
assertEquals(ROWS, cursors.size());
}
+
+@Test
+@WithSystemProperty(key = "IMPLICIT_PK_ENABLED", value = "true")
+//TODO: rewrite with QueryChecker after
+// https://issues.apache.org/jira/browse/IGNITE-18501 (QueryChecker still
use deprecated QueryProcessor#queryAsync method)
+public void testDifferentCollocatedSortAgg() {
+try {
+sql("CREATE TABLE testMe (a INTEGER, b INTEGER, s VARCHAR);");
+sql("INSERT INTO testMe VALUES (11, 1, 'hello'), (12, 2, 'world'),
(11, 3, NULL)");
+sql("INSERT INTO testMe VALUES (11, 3, 'hello'), (12, 2, 'world'),
(10, 5, 'ahello'), (13, 6, 'world')");
+
+String disabledRules = " /*+
DISABLE_RULE('MapReduceHashAggregateConverterRule',
'MapReduceSortAggregateConverterRule', "
++ "'ColocatedHashAggregateConverterRule') */ ";
+
+List> res = sql(appendDisabledRules("SELECT
DISTINCT(a) as a FROM testMe ORDER BY a", disabledRules));
+List res0 = transformResults(res);
+assertEquals(4, res.size());
+assertEquals(List.of(10, 11, 12, 13), res0);
+
+res = sql(appendDisabledRules("SELECT COUNT(*) FROM testMe",
disabledRules));
+res0 = transformResults(res);
+assertEquals(1, res.size());
+assertEquals(List.of(7L), res0);
+
+res = sql(appendDisabledRules("SELECT COUNT(a), COUNT(DISTINCT(b))
FROM testMe", disabledRules));
+
+assertEquals(1, res.size());
+assertEquals(List.of(7L, 5L), res.get(0));
+
+res = sql(appendDisabledRules("SELECT COUNT(a) as a, s FROM testMe
GROUP BY s ORDER BY a, s", disabledRules));
+res0 = transformResults(res);
+assertEquals(4, res.size());
+assertEquals(List.of(List.of(1L, "ahello"), Arrays.asList(1L,
null), List.of(2L, "hello"), List.of(3L, "world")), res0);
+
+res = sql(appendDisabledRules("SELECT COUNT(a) as a, AVG(a) as b,
MIN(a), MIN(b), s FROM testMe GROUP BY s ORDER BY a, b",
+disabledRules));
+res0 = transformResults(res);
+assertEquals(4, res.size());
+assertEquals(List.of(List.of(1L, 10, 10, 5, "ahello"),
Arrays.asList(1L, 11, 11, 3, null), List.of(2L, 11, 11, 1, "hello"),
+List.of(3L, 12, 12, 2, "world")), res0);
+
+res = sql(appendDisabledRules("SELECT COUNT(a) as a, AVG(a) as bb,
MIN(a), MIN(b), s FROM testMe GROUP BY s, b ORDER BY a, s",
+disabledRules));
+res0 = transformResults(res);
+assertEquals(6, res.size());
+assertEquals(List.of(List.of(1L, 10, 10, 5, "ahello"), List.of(1L,
11, 11, 1, "hello"), List.of(1L, 11, 11, 3, "hello"),
+List.of(1L, 13, 13, 6, "world"), Arrays.asList(1L, 11, 11,
3, null), List.of(2L, 12, 12, 2, "world")), res0);
+
+res = sql(appendDisabledRules("SELECT COUNT(a) FROM testMe",
disabledRules));
+res0 = transformResults(res);
+assertEquals(1, res.size());
+assertEquals(List.of(7L), res0);
+
+res = sql(appendDisabledRules("SELECT COUNT(DISTINCT(a)) FROM
testMe", disabledRules));
+res0 = transformResults(res);
+assertEquals(1, res.size());
+assertEquals(List.of(4L), res0);
+
+res = sql(appendDisabledRules("SELECT COUNT(a), COUNT(s), COUNT(*)
FROM testMe", disabledRules));
+res0 = transformResults(res);
+assertEquals(1, res.size());
+assertEquals(List.of(7L, 6L, 7L), res0.get(0));
+
+res = sql(appendDisabledRules("SELECT AVG(a) FROM testMe",
disabledRules));
+res0 = transformResults(res);
+assertEquals(1, res.size());
+assertEquals(List.of(11), res0);
+
+res = sql(appendDisabledRules("SELECT MIN(a) FROM testMe",
disabledRules));
+res0 = transformResults(res);
+assertEquals(1, res.size());
+assertEquals(List.of(10), res0);
+
+res = sql(appendDisabledRules("SELECT COUNT(a), COUNT(DISTINCT(a))
FROM testMe", disabledRules));
+
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083921930
##
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshotCommonTest.java:
##
@@ -76,12 +64,13 @@ void returnsKeyFromStorage() {
@Test
void sendsSnapshotMeta() {
-when(mvPartitionStorage.lastAppliedIndex()).thenReturn(100L);
-lenient().when(mvPartitionStorage.lastAppliedTerm()).thenReturn(3L);
-when(txStateStorage.lastAppliedIndex()).thenReturn(100L);
-lenient().when(txStateStorage.lastAppliedTerm()).thenReturn(3L);
+lenient().when(partitionAccess.minLastAppliedIndex()).thenReturn(90L);
Review Comment:
I tried to delete it, but got the following error: `Please remove
unnecessary stubbings or use 'lenient' strictness. More info: javadoc for
UnnecessaryStubbingException 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] nizhikov commented on a diff in pull request #10485: IGNITE-18232 : [IEP-80] Remove of CacheMetricsMXBean legacy JMX bean, v3 : keep JCache beans
nizhikov commented on code in PR #10485:
URL: https://github.com/apache/ignite/pull/10485#discussion_r1083923917
##
modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/RestMemcacheProtocolSelfTest.java:
##
@@ -185,8 +185,8 @@ public void testReplace() throws Exception {
*/
@Test
public void testMetrics() throws Exception {
-grid().cache(DEFAULT_CACHE_NAME).localMxBean().clear();
-grid().cache(CACHE_NAME).localMxBean().clear();
+grid().cache(DEFAULT_CACHE_NAME).clearStatistics();
Review Comment:
> This method calls GridCacheProcessor#clearStatistics().
How to do the same with public API?
--
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 #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083916394
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccess.java:
##
@@ -32,27 +41,111 @@ public interface PartitionAccess {
PartitionKey partitionKey();
/**
- * Returns the multi-versioned partition storage.
+ * Destroys and recreates the multi-versioned partition storage.
+ *
+ * @return Future that will complete when the partition is recreated.
+ * @throws StorageException If an error has occurred during the partition
destruction.
*/
-MvPartitionStorage mvPartitionStorage();
+CompletableFuture reCreateMvPartitionStorage() throws
StorageException;
/**
- * Returns transaction state storage for the partition.
+ * Destroys and recreates the multi-versioned partition storage.
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] tkalkirill commented on a diff in pull request #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083918501
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/incoming/IncomingSnapshotCopier.java:
##
@@ -186,32 +186,13 @@ public SnapshotReader getReader() {
/**
* Prepares the {@link MvPartitionStorage} for a full rebalance.
- *
- * Recreates {@link MvPartitionStorage} and sets the last applied index
to {@link TableManager#FULL_RABALANCING_STARTED} so that
- * when the node is restarted, we can understand that the full rebalance
has not completed, and we need to clean up the storage from
- * garbage.
*/
-private CompletableFuture
prepareMvPartitionStorageForRebalance(Executor executor) {
+private CompletableFuture prepareMvPartitionStorageForRebalance() {
if (canceled) {
return completedFuture(null);
}
-return
partitionSnapshotStorage.partition().reCreateMvPartitionStorage()
-.thenComposeAsync(mvPartitionStorage -> {
-if (canceled) {
-return completedFuture(null);
-}
-
-mvPartitionStorage.runConsistently(() -> {
-
mvPartitionStorage.lastApplied(FULL_RABALANCING_STARTED, 0);
-
-return null;
-});
-
-LOG.info("Copier prepared multi-versioned storage for the
partition [partId={}, tableId={}]", partId(), tableId());
-
-return completedFuture(null);
-}, executor);
+return
partitionSnapshotStorage.partition().reCreateMvPartitionStorage();
Review Comment:
This code will be removed soon, left TODO
--
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 #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
tkalkirill commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083919142
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/incoming/IncomingSnapshotCopier.java:
##
@@ -226,18 +207,7 @@ private CompletableFuture
prepareTxStatePartitionStorageForRebalance(Executor
return completedFuture(null);
}
-return CompletableFuture.supplyAsync(() ->
partitionSnapshotStorage.partition().reCreateTxStatePartitionStorage(),
executor)
-.thenCompose(txStatePartitionStorage -> {
-if (canceled) {
-return completedFuture(null);
-}
-
-
txStatePartitionStorage.lastApplied(FULL_RABALANCING_STARTED, 0);
-
-LOG.info("Copier prepared transaction state storage for
the partition [partId={}, tableId={}]", partId(), tableId());
Review Comment:
This code will be removed soon, left TODO
--
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_r1083913680
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSortAggregateTest.java:
##
@@ -67,8 +70,134 @@ public void mapReduceAggregate() {
@Test
public void correctCollationsOnMapReduceSortAgg() {
-var cursors = sql("SELECT PK FROM TEST_ONE_COL_IDX WHERE col0 IN
(SELECT col0 FROM TEST_ONE_COL_IDX)");
+String disabledRules = " /*+
DISABLE_RULE('MapReduceHashAggregateConverterRule',
'ColocatedSortAggregateConverterRule') */ ";
Review Comment:
the same question here
--
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_r1083913316
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItSortAggregateTest.java:
##
@@ -48,12 +53,10 @@ static void initTestData() {
@Test
public void mapReduceAggregate() {
+String disabledRules = " /*+
DISABLE_RULE('MapReduceHashAggregateConverterRule',
'ColocatedSortAggregateConverterRule') */ ";
Review Comment:
does it make sense to disable colocated hash aggregate rule as well?
--
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_r1083908015
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/SortAggregateNode.java:
##
@@ -119,7 +124,11 @@ public void push(RowT row) throws Exception {
waiting--;
if (grp != null) {
-int cmp = comp.compare(row, prevRow);
+int cmp = 0;
+
+if (comp != null) {
Review Comment:
comparator is never null
--
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_r1083907403
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/SortAggregateNode.java:
##
@@ -83,6 +84,10 @@ public SortAggregateNode(
this.rowFactory = rowFactory;
this.grpSet = grpSet;
this.comp = comp;
+
+if ((type == AggregateType.REDUCE || type == AggregateType.SINGLE) &&
accFactory != null && grpSet.isEmpty()) {
Review Comment:
what about rewind? (<-- let's add execution test for this case as well)
--
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 commented on a diff in pull request #1556: IGNITE-18593 Get rid of MvPartitionStorage and TxStateStorage in PartitionAccess
rpuch commented on code in PR #1556:
URL: https://github.com/apache/ignite-3/pull/1556#discussion_r1083873554
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccess.java:
##
@@ -32,27 +41,111 @@ public interface PartitionAccess {
PartitionKey partitionKey();
/**
- * Returns the multi-versioned partition storage.
+ * Destroys and recreates the multi-versioned partition storage.
+ *
+ * @return Future that will complete when the partition is recreated.
+ * @throws StorageException If an error has occurred during the partition
destruction.
*/
-MvPartitionStorage mvPartitionStorage();
+CompletableFuture reCreateMvPartitionStorage() throws
StorageException;
/**
- * Returns transaction state storage for the partition.
+ * Destroys and recreates the multi-versioned partition storage.
Review Comment:
```suggestion
* Destroys and recreates the TX state partition storage.
```
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionAccess.java:
##
@@ -32,27 +41,111 @@ public interface PartitionAccess {
PartitionKey partitionKey();
/**
- * Returns the multi-versioned partition storage.
+ * Destroys and recreates the multi-versioned partition storage.
+ *
+ * @return Future that will complete when the partition is recreated.
+ * @throws StorageException If an error has occurred during the partition
destruction.
*/
-MvPartitionStorage mvPartitionStorage();
+CompletableFuture reCreateMvPartitionStorage() throws
StorageException;
/**
- * Returns transaction state storage for the partition.
+ * Destroys and recreates the multi-versioned partition storage.
+ *
+ * @throws StorageException If an error has occurred during transaction
state storage for the partition destruction.
Review Comment:
```suggestion
* @throws StorageException If an error has occurred during destruction
of the transaction state storage for the partition.
```
##
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/snapshot/outgoing/OutgoingSnapshotCommonTest.java:
##
@@ -76,12 +64,13 @@ void returnsKeyFromStorage() {
@Test
void sendsSnapshotMeta() {
-when(mvPartitionStorage.lastAppliedIndex()).thenReturn(100L);
-lenient().when(mvPartitionStorage.lastAppliedTerm()).thenReturn(3L);
-when(txStateStorage.lastAppliedIndex()).thenReturn(100L);
-lenient().when(txStateStorage.lastAppliedTerm()).thenReturn(3L);
+lenient().when(partitionAccess.minLastAppliedIndex()).thenReturn(90L);
Review Comment:
These `lenient()`s seem redundant and should be dropped
##
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/snapshot/PartitionSnapshotStorageFactoryTest.java:
##
@@ -44,24 +39,18 @@
*/
@ExtendWith(MockitoExtension.class)
public class PartitionSnapshotStorageFactoryTest {
-private final MvPartitionStorage mvPartitionStorage = new
TestMvPartitionStorage(0);
-private final TxStateStorage txStateStorage = new TestTxStateStorage();
-
@Mock
private PartitionAccess partitionAccess;
-@BeforeEach
-void configureMocks() {
-
when(partitionAccess.mvPartitionStorage()).thenReturn(mvPartitionStorage);
-
when(partitionAccess.txStatePartitionStorage()).thenReturn(txStateStorage);
-}
-
@Test
void testForChoosingMinimumAppliedIndexForMeta() {
-mvPartitionStorage.lastApplied(10L, 2L);
-txStateStorage.lastApplied(5L, 1L);
+lenient().when(partitionAccess.minLastAppliedIndex()).thenReturn(5L);
Review Comment:
These `lenient()`s seem redundant (as the corresponding methods are always
called under this test), so they should be dropped.
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/incoming/IncomingSnapshotCopier.java:
##
@@ -226,18 +207,7 @@ private CompletableFuture
prepareTxStatePartitionStorageForRebalance(Executor
return completedFuture(null);
}
-return CompletableFuture.supplyAsync(() ->
partitionSnapshotStorage.partition().reCreateTxStatePartitionStorage(),
executor)
-.thenCompose(txStatePartitionStorage -> {
-if (canceled) {
-return completedFuture(null);
-}
-
-
txStatePartitionStorage.lastApplied(FULL_RABALANCING_STARTED, 0);
-
-LOG.info("Copier prepared transaction state storage for
the partition [partId={}, tableId={}]", partId(), tableId());
Review Comment:
Same thing: don't we want to log this anymore?
##
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/snapshot/incoming/IncomingSnapshotCopier.
[GitHub] [ignite-3] tkalkirill opened a new pull request, #1562: IGNITE-18030 Integration of the new full rebalance API with IncomingSnapshotCopier
tkalkirill opened a new pull request, #1562: URL: https://github.com/apache/ignite-3/pull/1562 https://issues.apache.org/jira/browse/IGNITE-18030 -- 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 merged pull request #1536: IGNITE-18207 Sql. Pushdown DISTINCT aggregate with no particular function
AMashenkov merged PR #1536: URL: https://github.com/apache/ignite-3/pull/1536 -- 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 #1469: IGNITE-18227: refactoring scan nodes and add support RO index scans.
korlov42 commented on code in PR #1469:
URL: https://github.com/apache/ignite-3/pull/1469#discussion_r1083836141
##
modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlAsynchronousApiTest.java:
##
@@ -343,37 +346,59 @@ 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);
+}
+
Review Comment:
extra line
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/SubscriptionUtils.java:
##
@@ -49,6 +50,31 @@ public static Publisher concat(Publisher... sources) {
return new ConcatenatedPublisher<>(Arrays.asList(sources).iterator());
}
+/**
+ * Create thread-safe publisher wrapper of combine multiple publishers,
sources for whom are suppliers what can be used for lazy
+ * opening these publishers . Generally, start consuming a source once the
previous source has terminated, building a chain.
+ *
+ * @param sources Array of publisher suppliers which should be combine.
+ * @return Publisher which will be combine all of passed as parameter to
single one.
+ */
+@SafeVarargs
+public static Publisher concat(Supplier>... sources) {
Review Comment:
I think, javadoc have to be reworked. Currently, there are a lot of grammar
mistakes, and it hard to understand. Besides, javadoc states this method
returns a thread-safe publisher, but non synchronised increment of non volatile
field is used to move to the next source...
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/IndexScanNode.java:
##
@@ -119,282 +91,122 @@ public IndexScanNode(
@Nullable Function rowTransformer,
@Nullable BitSet requiredColumns
) {
-super(ctx);
+super(ctx, rowFactory, schemaTable, filters, rowTransformer,
requiredColumns);
assert !nullOrEmpty(parts);
-
-assert ctx.transaction() != null || ctx.transactionTime() != null :
"Transaction not initialized.";
+assert rangeConditions == null || rangeConditions.size() > 0;
this.schemaIndex = schemaIndex;
this.parts = parts;
-this.filters = filters;
-this.rowTransformer = rowTransformer;
this.requiredColumns = requiredColumns;
this.rangeConditions = rangeConditions;
this.comp = comp;
this.factory = rowFactory;
-rangeConditionIterator = rangeConditions == null ? null :
rangeConditions.iterator();
-
-tableRowConverter = row -> schemaTable.toRow(context(), row, factory,
requiredColumns);
-
indexRowSchema =
RowConverter.createIndexRowSchema(schemaIndex.columns(),
schemaTable.descriptor());
}
/** {@inheritDoc} */
@Override
-public void request(int rowsCnt) throws Exception {
-assert rowsCnt > 0 && requested == 0 : "rowsCnt=" + rowsCnt + ",
requested=" + requested;
-
-checkState();
-
-requested = rowsCnt;
-
-if (!inLoop) {
-context().execute(this::push, this::onError);
-}
-}
-
-/** {@inheritDoc} */
-@Override
-public void closeInternal() {
-super.closeInternal();
-
-if (activeSubscription != null) {
-activeSubscription.cancel();
-
-activeSubscription = null;
-}
-}
-
-/** {@inheritDoc} */
-@Override
-protected void rewindInternal() {
-requested = 0;
-waiting = 0;
-rangeConditionsProcessed = false;
-
+protected Publisher scan() {
if (rangeConditions != null) {
-rangeConditionIterator = rangeConditions.iterator();
-}
-
-if (activeSubscription != null) {
-activeSubscription.cancel();
-
-activeSubscription = null;
-}
-}
-
-/** {@inheritDoc} */
-@Override
-public void register(List> sources) {
-throw new UnsupportedOperationException();
-}
-
-/** {@inheritDoc} */
-@Override
-protected Downstream requestDownstream(int idx) {
-throw new UnsupportedOperationException();
-}
-
-private void push() throws Exception {
-if (isClosed()) {
-return;
-}
-
-checkState();
-
-if (requested > 0 && !inBuff.isEmpty()) {
-inLoop = true;
-try {
-while (requested > 0 && !inBuff.isEmpty()) {
-checkState();
-
-Ro
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10485: IGNITE-18232 : [IEP-80] Remove of CacheMetricsMXBean legacy JMX bean, v3 : keep JCache beans
Vladsz83 commented on code in PR #10485: URL: https://github.com/apache/ignite/pull/10485#discussion_r1083841496 ## modules/core/src/main/java/org/apache/ignite/IgniteCache.java: ## @@ -1617,20 +1616,6 @@ public IgniteFuture>> invokeAllAsync(Set
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10485: IGNITE-18232 : [IEP-80] Remove of CacheMetricsMXBean legacy JMX bean, v3 : keep JCache beans
Vladsz83 commented on code in PR #10485:
URL: https://github.com/apache/ignite/pull/10485#discussion_r1083835143
##
modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/RestMemcacheProtocolSelfTest.java:
##
@@ -185,8 +185,8 @@ public void testReplace() throws Exception {
*/
@Test
public void testMetrics() throws Exception {
-grid().cache(DEFAULT_CACHE_NAME).localMxBean().clear();
-grid().cache(CACHE_NAME).localMxBean().clear();
+grid().cache(DEFAULT_CACHE_NAME).clearStatistics();
Review Comment:
> Why do we need new method `clearStatistics`? AFAICU clear method comes
from `javax.cache.management.CacheStatisticsMXBean`
This method calls GridCacheProcessor#clearStatistics(). It isn't new. I
brought other call in the tests to keep the conception of mxbean removal.
We keep them now only to support JCache spec. I suggest not to use JCache
mxbean interfaces any more even in the tests. 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] nizhikov commented on a diff in pull request #10485: IGNITE-18232 : [IEP-80] Remove of CacheMetricsMXBean legacy JMX bean, v3 : keep JCache beans
nizhikov commented on code in PR #10485: URL: https://github.com/apache/ignite/pull/10485#discussion_r1083760009 ## modules/core/src/main/java/org/apache/ignite/IgniteCache.java: ## @@ -1617,20 +1616,6 @@ public IgniteFuture>> invokeAllAsync(Set
[GitHub] [ignite] nizhikov commented on a diff in pull request #10485: IGNITE-18232 : [IEP-80] Remove of CacheMetricsMXBean legacy JMX bean, v3 : keep JCache beans
nizhikov commented on code in PR #10485:
URL: https://github.com/apache/ignite/pull/10485#discussion_r1083757319
##
modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/RestMemcacheProtocolSelfTest.java:
##
@@ -185,8 +185,8 @@ public void testReplace() throws Exception {
*/
@Test
public void testMetrics() throws Exception {
-grid().cache(DEFAULT_CACHE_NAME).localMxBean().clear();
-grid().cache(CACHE_NAME).localMxBean().clear();
+grid().cache(DEFAULT_CACHE_NAME).clearStatistics();
Review Comment:
Why do we need new method `clearStatistics`?
AFAICU clear method comes from `javax.cache.management.CacheStatisticsMXBean`
Can we just invoke method from MX bean instance as we do now?
--
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 #1479: IGNITE-18360 Migrate storage to new Binary Tuple format
ibessonov commented on code in PR #1479:
URL: https://github.com/apache/ignite-3/pull/1479#discussion_r1083750250
##
modules/schema/src/main/java/org/apache/ignite/internal/schema/TableRow.java:
##
@@ -0,0 +1,105 @@
+/*
+ * 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.schema;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+
+/** Heap byte buffer-based row. */
+public class TableRow {
+public static final ByteOrder ORDER = ByteOrder.LITTLE_ENDIAN;
+
+/** Size of schema version length field. */
+private static final int SCHEMA_VERSION_FLD_LEN = Short.BYTES;
+
+/** Row schema version field offset. */
+private static final int SCHEMA_VERSION_OFFSET = 0;
+
+/** Row binary tuple field offset. */
+private static final int TUPLE_OFFSET = SCHEMA_VERSION_OFFSET +
SCHEMA_VERSION_FLD_LEN;
+
+/** Row buffer. */
+private final ByteBuffer buf;
+
+/**
+ * Constructor.
+ *
+ * @param data Array representation of the row.
+ */
+public TableRow(byte[] data) {
+this(ByteBuffer.wrap(data).order(ORDER));
+}
+
+/**
+ * Constructor.
+ *
+ * @param buf Buffer representing the row.
+ */
+public TableRow(ByteBuffer buf) {
+assert buf.order() == ORDER;
+assert buf.position() == 0;
+
+this.buf = buf;
+}
+
+/**
+ * Factory method which creates an instance using a schema version and an
existing {@link BinaryTuple}.
+ *
+ * @param schemaVersion Schema version.
+ * @param binaryTuple Binary tuple.
+ * @return Created TableRow instance.
+ */
+public static TableRow createFromTuple(int schemaVersion, BinaryTuple
binaryTuple) {
Review Comment:
Ok, but we should fix this in the future. There must be a TODO. Current
approach is not efficient
--
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
