Re: [PR] IGNITE-21435: Sql. Catalog DefaultValue should not depend on Serializable. [ignite-3]
zstan merged PR #3627: URL: https://github.com/apache/ignite-3/pull/3627 -- 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
Re: [PR] IGNITE-21937 Sql. Cover SQL F041-05 feature by tests [ignite-3]
zstan merged PR #3643: URL: https://github.com/apache/ignite-3/pull/3643 -- 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
[PR] IGNITE-22079 Upgrade spring data extension to spring 6. [ignite-extensions]
shnus opened a new pull request, #264: URL: https://github.com/apache/ignite-extensions/pull/264 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1576570458
##
modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/ItSimpleCounterServerTest.java:
##
@@ -97,14 +99,16 @@ void before() throws Exception {
server = new JraftServerImpl(service, workDir, raftConfiguration) {
@Override
-public synchronized void stop() throws Exception {
-super.stop();
+public synchronized CompletableFuture stopAsync() {
Review Comment:
Looks like it's a leftover from the past times. removed 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1576566401
##
modules/network/src/testFixtures/java/org/apache/ignite/internal/network/utils/ClusterServiceTestUtils.java:
##
@@ -178,24 +179,26 @@ public CompletableFuture start() {
)
).join();
-bootstrapFactory.start();
+bootstrapFactory.startAsync();
-clusterSvc.start();
+clusterSvc.startAsync();
return nullCompletedFuture();
}
@Override
-public void stop() {
+public CompletableFuture stopAsync() {
try {
IgniteUtils.closeAll(
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1576566038
##
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerSelfTest.java:
##
@@ -239,9 +239,9 @@ public void testNoInteractionsAfterStop() throws Exception {
CompletableFuture readyFuture = manager.catalogReadyFuture(1);
assertFalse(readyFuture.isDone());
-manager.stop();
+manager.stopAsync();
Review Comment:
fixed
##
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogTestUtilsTest.java:
##
@@ -80,7 +80,7 @@ void testManagerWorksAsExpected() throws Exception {
assertThat(tablesOfVersion2, hasItem(descriptorWithName("T1")));
assertThat(tablesOfVersion2, hasItem(descriptorWithName("T2")));
-manager.stop();
+manager.stopAsync();
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
Cyrill commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1576565805
##
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java:
##
@@ -191,16 +191,18 @@ public CompletableFuture start() {
updateLog.registerUpdateHandler(new OnUpdateHandlerImpl());
-updateLog.start();
+updateLog.startAsync();
return nullCompletedFuture();
}
@Override
-public void stop() throws Exception {
+public CompletableFuture stopAsync() {
busyLock.block();
versionTracker.close();
-updateLog.stop();
+updateLog.stopAsync();
Review Comment:
fixed
##
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java:
##
@@ -191,16 +191,18 @@ public CompletableFuture start() {
updateLog.registerUpdateHandler(new OnUpdateHandlerImpl());
-updateLog.start();
+updateLog.startAsync();
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] IGNITE-22078 Upgrade spring tx extension to spring 6. [ignite-extensions]
shnus opened a new pull request, #263: URL: https://github.com/apache/ignite-extensions/pull/263 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] IGNITE-22095 Remove compute job jars from the repo [ignite-3]
valepakh opened a new pull request, #3655: URL: https://github.com/apache/ignite-3/pull/3655 https://issues.apache.org/jira/browse/IGNITE-22095 -- 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
Re: [PR] Bump org.rocksdb:rocksdbjni from 8.11.3 to 9.0.1 [ignite-3]
dependabot[bot] closed pull request #3649: Bump org.rocksdb:rocksdbjni from 8.11.3 to 9.0.1 URL: https://github.com/apache/ignite-3/pull/3649 -- 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
Re: [PR] Bump org.rocksdb:rocksdbjni from 8.11.3 to 9.0.1 [ignite-3]
dependabot[bot] commented on PR #3649: URL: https://github.com/apache/ignite-3/pull/3649#issuecomment-2072648500 Superseded by #3654. -- 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
[PR] Bump org.rocksdb:rocksdbjni from 8.11.3 to 9.1.1 [ignite-3]
dependabot[bot] opened a new pull request, #3654: URL: https://github.com/apache/ignite-3/pull/3654 Bumps [org.rocksdb:rocksdbjni](https://github.com/facebook/rocksdb) from 8.11.3 to 9.1.1. Release notes Sourced from https://github.com/facebook/rocksdb/releases;>org.rocksdb:rocksdbjni's releases. RocksDB 9.1.1 9.1.1 (2024-04-17) Bug Fixes Fixed Java SstFileMetaData to prevent throwing java.lang.NoSuchMethodError Fixed a regression when ColumnFamilyOptions::max_successive_merges 0 where the CPU overhead for deciding whether to merge could have increased unless the user had set the option ColumnFamilyOptions::strict_max_successive_merges RocksDB 9.1.0 9.1.0 (2024-03-22) New Features Added an option, GetMergeOperandsOptions::continue_cb, to give users the ability to end GetMergeOperands()'s lookup process before all merge operands were found. *Add sanity checks for ingesting external files that currently checks if the user key comparator used to create the file is compatible with the column family's user key comparator. *Support ingesting external files for column family that has user-defined timestamps in memtable only enabled. On file systems that support storage level data checksum and reconstruction, retry SST block reads for point lookups, scans, and flush and compaction if there's a checksum mismatch on the initial read. Some enhancements and fixes to experimental Temperature handling features, including new default_write_temperature CF option and opening an SstFileWriter with a temperature. WriteBatchWithIndex now supports wide-column point lookups via the GetEntityFromBatch API. See the API comments for more details. *Implement experimental features: API Iterator::GetProperty(rocksdb.iterator.write-time) to allow users to get data's approximate write unix time and write data with a specific write time via WriteBatch::TimedPut API. Public API Changes Best-effort recovery (best_efforts_recovery == true) may now be used together with atomic flush (atomic_flush == true). The all-or-nothing recovery guarantee for atomically flushed data will be upheld. Remove deprecated option bottommost_temperature, already replaced by last_level_temperature Added new PerfContext counters for block cache bytes read - block_cache_index_read_byte, block_cache_filter_read_byte, block_cache_compression_dict_read_byte, and block_cache_read_byte. Deprecate experimental Remote Compaction APIs - StartV2() and WaitForCompleteV2() and introduce Schedule() and Wait(). The new APIs essentially does the same thing as the old APIs. They allow taking externally generated unique id to wait for remote compaction to complete. *For API WriteCommittedTransaction::GetForUpdate, if the column family enables user-defined timestamp, it was mandated that argument do_validate cannot be false, and UDT based validation has to be done with a user set read timestamp. It's updated to make the UDT based validation optional if user sets do_validate to false and does not set a read timestamp. With this, GetForUpdate skips UDT based validation and it's users' responsibility to enforce the UDT invariant. SO DO NOT skip this UDT-based validation if users do not have ways to enforce the UDT invariant. Ways to enforce the invariant on the users side include manage a monotonically increasing timestamp, commit transactions in a single thread etc. Defined a new PerfLevel kEnableWait to measure time spent by user threads blocked in RocksDB other than mutex, such as a write thread waiting to be added to a write group, a write thread delayed or stalled etc. RateLimiter's API no longer requires the burst size to be the refill size. Users of NewGenericRateLimiter() can now provide burst size in single_burst_bytes. Implementors of RateLimiter::SetSingleBurstBytes() need to adapt their implementations to match the changed API doc. Add write_memtable_time to the newly introduced PerfLevel kEnableWait. Behavior Changes RateLimiters created by NewGenericRateLimiter() no longer modify the refill period when SetSingleBurstBytes() is called. Merge writes will only keep merge operand count within ColumnFamilyOptions::max_successive_merges when the key's merge operands are all found in memory, unless strict_max_successive_merges is explicitly set. Bug Fixes Fixed kBlockCacheTier reads to return Status::Incomplete when I/O is needed to fetch a merge chain's base value from a blob file. Fixed kBlockCacheTier reads to return Status::Incomplete on table cache miss rather than incorrectly returning an empty value. Fixed a data race in WalManager that may affect how frequent PurgeObsoleteWALFiles() runs. Re-enable the recycle_log_file_num option in DBOptions for kPointInTimeRecovery WAL recovery mode, which was previously disabled due to a bug in the recovery logic. This
Re: [PR] IGNITE-21720 Sql. Implement hash join [ignite-3]
zstan commented on code in PR #3608:
URL: https://github.com/apache/ignite-3/pull/3608#discussion_r1576438409
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/HashJoinNode.java:
##
@@ -0,0 +1,623 @@
+/*
+ * 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.sql.engine.util.TypeUtils.rowSchemaFromRelTypes;
+
+import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.JoinInfo;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.ignite.internal.sql.engine.exec.ExecutionContext;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.jetbrains.annotations.Nullable;
+
+/** HashJoin implementor. */
+public abstract class HashJoinNode extends
AbstractRightMaterializedJoinNode {
+Map hashStore = new Object2ObjectOpenHashMap<>();
+protected final RowHandler handler;
+
+final Collection leftJoinPositions;
+private final Collection rightJoinPositions;
+
+final boolean touchResults;
+
+Iterator rightIt = Collections.emptyIterator();
+
+private HashJoinNode(ExecutionContext ctx, JoinInfo joinInfo,
boolean touch) {
+super(ctx);
+
+handler = ctx.rowHandler();
+touchResults = touch;
+
+leftJoinPositions = joinInfo.leftKeys.toIntegerList();
+rightJoinPositions = joinInfo.rightKeys.toIntegerList();
+}
+
+@Override
+protected void rewindInternal() {
+rightIt = Collections.emptyIterator();
+
+hashStore.clear();
+
+super.rewindInternal();
+}
+
+/** Supplied algorithm implementation. */
+public static HashJoinNode create(ExecutionContext ctx,
RelDataType outputRowType,
+RelDataType leftRowType, RelDataType rightRowType, JoinRelType
joinType, JoinInfo joinInfo) {
+
+switch (joinType) {
+case INNER:
+return new InnerHashJoin<>(ctx, joinInfo);
+
+case LEFT: {
+RowSchema rightRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(rightRowType));
+RowHandler.RowFactory rightRowFactory =
ctx.rowHandler().factory(rightRowSchema);
+
+return new LeftHashJoin<>(ctx, rightRowFactory, joinInfo);
+}
+
+case RIGHT: {
+RowSchema leftRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(leftRowType));
+RowHandler.RowFactory leftRowFactory =
ctx.rowHandler().factory(leftRowSchema);
+
+return new RightHashJoin<>(ctx, leftRowFactory, joinInfo);
+}
+
+case FULL: {
+RowSchema leftRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(leftRowType));
+RowSchema rightRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(rightRowType));
+RowHandler.RowFactory leftRowFactory =
ctx.rowHandler().factory(leftRowSchema);
+RowHandler.RowFactory rightRowFactory =
ctx.rowHandler().factory(rightRowSchema);
+
+return new FullOuterHashJoin<>(ctx, leftRowFactory,
rightRowFactory, joinInfo);
+}
+
+case SEMI:
+return new SemiHashJoin<>(ctx, joinInfo);
+
+case ANTI:
+return new AntiHashJoin<>(ctx, joinInfo);
+
+default:
+throw new IllegalStateException("Join type \"" + joinType +
"\" is not supported yet");
+}
+}
+
+private static class InnerHashJoin extends HashJoinNode {
+private InnerHashJoin(ExecutionContext ctx, JoinInfo joinInfo) {
+super(ctx, joinInfo, false);
+}
+
+@Override
+protected void join() throws Exception {
+if (waitingRight == NOT_WAITING) {
+
Re: [I] IBinarizable : Mapping complex objects [ignite]
satishviswanathan commented on issue #11325: URL: https://github.com/apache/ignite/issues/11325#issuecomment-2072605768 @ptupitsyn Thank you for your response. This is what I have been trying to do. DDL statement. I have created the following table by executing the DDL statement from Database Manager (DBeaver). ``` CREATE TABLE IF NOT EXISTS employee ( id int, name varchar, companyid varchar, age int, city varchar, street varchar, streetnumber int, flatnumber int, PRIMARY KEY (id) ) WITH "template=partitioned,backups=1,WRITE_SYNCHRONIZATION_MODE=FULL_SYNC,CACHE_GROUP=Employee,CACHE_NAME=Employee,KEY_TYPE=int,VALUE_TYPE=Employee"; ``` _After that I have executed the following program to create an employee record , however the address details are not getting saved_ [ignite-sample.zip](https://github.com/apache/ignite/files/15078729/ignite-sample.zip) _Employee Table view_  -- 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
[PR] IGNITE-21859 Causality token stays 0 for default zone [ignite-3]
korlov42 opened a new pull request, #3653: URL: https://github.com/apache/ignite-3/pull/3653 Thank you for submitting the pull request. To streamline the review process of the patch and ensure better code quality we ask both an author and a reviewer to verify the following: ### The Review Checklist - [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following: \- There is a single JIRA ticket related to the pull request. \- The web-link to the pull request is attached to the JIRA ticket. \- The JIRA ticket has the Patch Available state. \- The description of the JIRA ticket explains WHAT was made, WHY and HOW. \- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE- Change summary where - number of JIRA issue. - [ ] **Design:** new code conforms with the design principles of the components it is added to. - [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable. - [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed. - [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources. ### Notes - [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-21720 Sql. Implement hash join [ignite-3]
zstan commented on code in PR #3608:
URL: https://github.com/apache/ignite-3/pull/3608#discussion_r1576276569
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/HashJoinNode.java:
##
@@ -0,0 +1,623 @@
+/*
+ * 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.sql.engine.util.TypeUtils.rowSchemaFromRelTypes;
+
+import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.JoinInfo;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.ignite.internal.sql.engine.exec.ExecutionContext;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.jetbrains.annotations.Nullable;
+
+/** HashJoin implementor. */
+public abstract class HashJoinNode extends
AbstractRightMaterializedJoinNode {
+Map hashStore = new Object2ObjectOpenHashMap<>();
+protected final RowHandler handler;
+
+final Collection leftJoinPositions;
+private final Collection rightJoinPositions;
+
+final boolean touchResults;
+
+Iterator rightIt = Collections.emptyIterator();
+
+private HashJoinNode(ExecutionContext ctx, JoinInfo joinInfo,
boolean touch) {
+super(ctx);
+
+handler = ctx.rowHandler();
+touchResults = touch;
+
+leftJoinPositions = joinInfo.leftKeys.toIntegerList();
+rightJoinPositions = joinInfo.rightKeys.toIntegerList();
+}
+
+@Override
+protected void rewindInternal() {
+rightIt = Collections.emptyIterator();
+
+hashStore.clear();
+
+super.rewindInternal();
+}
+
+/** Supplied algorithm implementation. */
+public static HashJoinNode create(ExecutionContext ctx,
RelDataType outputRowType,
+RelDataType leftRowType, RelDataType rightRowType, JoinRelType
joinType, JoinInfo joinInfo) {
+
+switch (joinType) {
+case INNER:
+return new InnerHashJoin<>(ctx, joinInfo);
+
+case LEFT: {
+RowSchema rightRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(rightRowType));
+RowHandler.RowFactory rightRowFactory =
ctx.rowHandler().factory(rightRowSchema);
+
+return new LeftHashJoin<>(ctx, rightRowFactory, joinInfo);
+}
+
+case RIGHT: {
+RowSchema leftRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(leftRowType));
+RowHandler.RowFactory leftRowFactory =
ctx.rowHandler().factory(leftRowSchema);
+
+return new RightHashJoin<>(ctx, leftRowFactory, joinInfo);
+}
+
+case FULL: {
+RowSchema leftRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(leftRowType));
+RowSchema rightRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(rightRowType));
+RowHandler.RowFactory leftRowFactory =
ctx.rowHandler().factory(leftRowSchema);
+RowHandler.RowFactory rightRowFactory =
ctx.rowHandler().factory(rightRowSchema);
+
+return new FullOuterHashJoin<>(ctx, leftRowFactory,
rightRowFactory, joinInfo);
+}
+
+case SEMI:
+return new SemiHashJoin<>(ctx, joinInfo);
+
+case ANTI:
+return new AntiHashJoin<>(ctx, joinInfo);
+
+default:
+throw new IllegalStateException("Join type \"" + joinType +
"\" is not supported yet");
+}
+}
+
+private static class InnerHashJoin extends HashJoinNode {
+private InnerHashJoin(ExecutionContext ctx, JoinInfo joinInfo) {
+super(ctx, joinInfo, false);
+}
+
+@Override
+protected void join() throws Exception {
+if (waitingRight == NOT_WAITING) {
+
Re: [PR] IGNITE-21720 Sql. Implement hash join [ignite-3]
zstan commented on code in PR #3608:
URL: https://github.com/apache/ignite-3/pull/3608#discussion_r1576276053
##
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/rel/HashJoinNode.java:
##
@@ -0,0 +1,623 @@
+/*
+ * 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.sql.engine.util.TypeUtils.rowSchemaFromRelTypes;
+
+import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.core.JoinInfo;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.ignite.internal.sql.engine.exec.ExecutionContext;
+import org.apache.ignite.internal.sql.engine.exec.RowHandler;
+import org.apache.ignite.internal.sql.engine.exec.row.RowSchema;
+import org.jetbrains.annotations.Nullable;
+
+/** HashJoin implementor. */
+public abstract class HashJoinNode extends
AbstractRightMaterializedJoinNode {
+Map hashStore = new Object2ObjectOpenHashMap<>();
+protected final RowHandler handler;
+
+final Collection leftJoinPositions;
+private final Collection rightJoinPositions;
+
+final boolean touchResults;
+
+Iterator rightIt = Collections.emptyIterator();
+
+private HashJoinNode(ExecutionContext ctx, JoinInfo joinInfo,
boolean touch) {
+super(ctx);
+
+handler = ctx.rowHandler();
+touchResults = touch;
+
+leftJoinPositions = joinInfo.leftKeys.toIntegerList();
+rightJoinPositions = joinInfo.rightKeys.toIntegerList();
+}
+
+@Override
+protected void rewindInternal() {
+rightIt = Collections.emptyIterator();
+
+hashStore.clear();
+
+super.rewindInternal();
+}
+
+/** Supplied algorithm implementation. */
+public static HashJoinNode create(ExecutionContext ctx,
RelDataType outputRowType,
+RelDataType leftRowType, RelDataType rightRowType, JoinRelType
joinType, JoinInfo joinInfo) {
+
+switch (joinType) {
+case INNER:
+return new InnerHashJoin<>(ctx, joinInfo);
+
+case LEFT: {
+RowSchema rightRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(rightRowType));
+RowHandler.RowFactory rightRowFactory =
ctx.rowHandler().factory(rightRowSchema);
+
+return new LeftHashJoin<>(ctx, rightRowFactory, joinInfo);
+}
+
+case RIGHT: {
+RowSchema leftRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(leftRowType));
+RowHandler.RowFactory leftRowFactory =
ctx.rowHandler().factory(leftRowSchema);
+
+return new RightHashJoin<>(ctx, leftRowFactory, joinInfo);
+}
+
+case FULL: {
+RowSchema leftRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(leftRowType));
+RowSchema rightRowSchema =
rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(rightRowType));
+RowHandler.RowFactory leftRowFactory =
ctx.rowHandler().factory(leftRowSchema);
+RowHandler.RowFactory rightRowFactory =
ctx.rowHandler().factory(rightRowSchema);
+
+return new FullOuterHashJoin<>(ctx, leftRowFactory,
rightRowFactory, joinInfo);
+}
+
+case SEMI:
+return new SemiHashJoin<>(ctx, joinInfo);
+
+case ANTI:
+return new AntiHashJoin<>(ctx, joinInfo);
+
+default:
+throw new IllegalStateException("Join type \"" + joinType +
"\" is not supported yet");
+}
+}
+
+private static class InnerHashJoin extends HashJoinNode {
+private InnerHashJoin(ExecutionContext ctx, JoinInfo joinInfo) {
+super(ctx, joinInfo, false);
+}
+
+@Override
+protected void join() throws Exception {
+if (waitingRight == NOT_WAITING) {
+
Re: [PR] IGNITE-21720 Sql. Implement hash join [ignite-3]
zstan commented on code in PR #3608:
URL: https://github.com/apache/ignite-3/pull/3608#discussion_r1576268912
##
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItJoinTest.java:
##
@@ -843,6 +1067,7 @@ private static Stream joinTypes() {
Stream types = Arrays.stream(JoinType.values())
// TODO: https://issues.apache.org/jira/browse/IGNITE-21286
remove filter below
.filter(type -> type != JoinType.CORRELATED)
+.filter(type -> type != JoinType.HASHJOIN)
Review Comment:
done, thanks
--
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
Re: [PR] [IGNITE-21999] Merge partition free-lists into one [ignite-3]
ibessonov commented on code in PR #3615:
URL: https://github.com/apache/ignite-3/pull/3615#discussion_r1576172447
##
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/Storable.java:
##
@@ -59,5 +67,27 @@ public interface Storable {
/**
* Returns I/O for handling this storable.
*/
-IoVersions> ioVersions();
+IoVersions ioVersions();
+
+/** Returns a byte buffer that contains binary tuple data. */
Review Comment:
Excuse me, what binary tuple? It's an abstraction, it should not depend on
the implementation. There's clearly something wrong here.
What was wrong with `writeRowData` and `writeFragmentData` that you could
have moved here?
##
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/freelist/FreeList.java:
##
@@ -21,30 +21,27 @@
import org.apache.ignite.internal.lang.IgniteInternalCheckedException;
import org.apache.ignite.internal.logger.IgniteLogger;
import org.apache.ignite.internal.pagememory.Storable;
-import org.apache.ignite.internal.pagememory.metric.IoStatisticsHolder;
import org.apache.ignite.internal.pagememory.util.PageHandler;
/**
* Free list.
*/
-public interface FreeList {
+public interface FreeList {
/**
* Inserts a row.
*
* @param row Row.
- * @param statHolder Statistics holder to track IO operations.
* @throws IgniteInternalCheckedException If failed.
*/
-void insertDataRow(T row, IoStatisticsHolder statHolder) throws
IgniteInternalCheckedException;
+void insertDataRow(Storable row) throws IgniteInternalCheckedException;
Review Comment:
Why did you remove stat holder? Let's leave it for now. If you want to clean
the code, then please do it in a separate PR, this one is already big enough.
##
modules/core/src/main/java/org/apache/ignite/internal/util/CachedIterator.java:
##
@@ -0,0 +1,47 @@
+/*
+ * 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.util;
+
+import java.util.Iterator;
+
+/** {@link Iterator} implementation that allows to access the current element
multiple times. */
+public class CachedIterator implements Iterator {
Review Comment:
Maybe we shouldn't put it in `core`, this class is very specific. `get`
contract is weird and hard to explain, so I'd rather hide it
##
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/freelist/FreeListImpl.java:
##
@@ -886,9 +614,28 @@ public long recycledPagesCount() throws
IgniteInternalCheckedException {
}
}
-/** {@inheritDoc} */
+@Override
+public void saveMetadata() throws IgniteInternalCheckedException {
+saveMetadata(statHolder);
+}
+
+/** Returns page eviction tracker. */
+public PageEvictionTracker evictionTracker() {
+return evictionTracker;
+}
+
+/** Returns page memory. */
+public PageMemory pageMemory() {
+return pageMem;
+}
+
+/** Returns write row handler. */
+public WriteRowHandler writeRowHandler() {
Review Comment:
This is weird, why did you make it a method? Artifact of the refactoring?
I would recommend you to rollback some of the changes.
##
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/freelist/FreeListImpl.java:
##
@@ -84,300 +86,59 @@ public abstract class AbstractFreeList
extends PagesList imp
protected final PageEvictionTracker evictionTracker;
/** Write a single row on a single page. */
-private final WriteRowHandler writeRowHnd = new WriteRowHandler();
+private final WriteRowHandler writeRowHnd = new WriteRowHandler(this);
/** Write multiple rows on a single page. */
-private final WriteRowsHandler writeRowsHnd = new WriteRowsHandler();
+private final WriteRowsHandler writeRowsHnd = new WriteRowsHandler(this);
private final PageHandler rmvRow;
-private class WriteRowHandler implements PageHandler {
-/** {@inheritDoc} */
-@Override
-public Integer run(
-int cacheId,
-long pageId,
-long page,
-long pageAddr,
-
[PR] IGNITE-22093: Sql. Rename PlannerPhase::HEP_DECORRELATE [ignite-3]
lowka opened a new pull request, #3652: URL: https://github.com/apache/ignite-3/pull/3652 https://issues.apache.org/jira/browse/IGNITE-22093 --- Thank you for submitting the pull request. To streamline the review process of the patch and ensure better code quality we ask both an author and a reviewer to verify the following: ### The Review Checklist - [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following: \- There is a single JIRA ticket related to the pull request. \- The web-link to the pull request is attached to the JIRA ticket. \- The JIRA ticket has the Patch Available state. \- The description of the JIRA ticket explains WHAT was made, WHY and HOW. \- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE- Change summary where - number of JIRA issue. - [ ] **Design:** new code conforms with the design principles of the components it is added to. - [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable. - [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed. - [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources. ### Notes - [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1576130991
##
modules/raft/src/integrationTest/java/org/apache/ignite/raft/server/ItSimpleCounterServerTest.java:
##
@@ -97,14 +99,16 @@ void before() throws Exception {
server = new JraftServerImpl(service, workDir, raftConfiguration) {
@Override
-public synchronized void stop() throws Exception {
-super.stop();
+public synchronized CompletableFuture stopAsync() {
Review Comment:
Curious whether synchronized was expected to cover all stop actions?
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1576119948
##
modules/network/src/testFixtures/java/org/apache/ignite/internal/network/utils/ClusterServiceTestUtils.java:
##
@@ -178,24 +179,26 @@ public CompletableFuture start() {
)
).join();
-bootstrapFactory.start();
+bootstrapFactory.startAsync();
-clusterSvc.start();
+clusterSvc.startAsync();
return nullCompletedFuture();
}
@Override
-public void stop() {
+public CompletableFuture stopAsync() {
try {
IgniteUtils.closeAll(
Review Comment:
How are we going to handle async exceptions inside closeAll? I mean that it
was designed to close as much as possible, semi-ignoring pending exceptions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] IGNITE-22024 Fixed ItSqlClientSynchronousApiTest#runtimeErrorInDmlCausesTransactionToFail [ignite-3]
vldpyatkov merged PR #3651: URL: https://github.com/apache/ignite-3/pull/3651 -- 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
Re: [PR] IGNITE-21805 Refactor TableManager and move all RAFT related pieces to Replica [ignite-3]
alievmirza commented on code in PR #3633:
URL: https://github.com/apache/ignite-3/pull/3633#discussion_r1576080458
##
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##
@@ -493,45 +612,92 @@ public CompletableFuture startReplica(
}
}
+private CompletableFuture startReplicaInternal(
+ReplicationGroupId replicaGrpId,
+ReplicaListener listener,
+RaftGroupService raftClient,
+PendingComparableValuesTracker storageIndexTracker
+) throws NodeStoppingException {
+LOG.info("Replica is about to start [replicationGroupId={}].",
replicaGrpId);
+
+ClusterNode localNode = clusterNetSvc.topologyService().localMember();
+
+Replica newReplica = new Replica(
+replicaGrpId,
+listener,
+storageIndexTracker,
+localNode,
+executor,
+placementDriver,
+clockService);
+
+CompletableFuture replicaFuture =
replicas.compute(replicaGrpId, (k, existingReplicaFuture) -> {
+if (existingReplicaFuture == null ||
existingReplicaFuture.isDone()) {
+assert existingReplicaFuture == null ||
isCompletedSuccessfully(existingReplicaFuture);
+LOG.info("Replica is started [replicationGroupId={}].",
replicaGrpId);
+
+return CompletableFuture.completedFuture(newReplica);
+} else {
+existingReplicaFuture.complete(newReplica);
+LOG.info("Replica is started, existing replica waiter was
completed [replicationGroupId={}].", replicaGrpId);
+
+return existingReplicaFuture;
+}
+});
+
+var eventParams = new LocalReplicaEventParameters(replicaGrpId);
+
+return fireEvent(AFTER_REPLICA_STARTED, eventParams)
+.exceptionally(e -> {
+LOG.error("Error when notifying about
AFTER_REPLICA_STARTED event.", e);
+
+return null;
+})
+.thenCompose(v -> replicaFuture);
+}
+
/**
* Internal method for starting a replica.
*
* @param replicaGrpId Replication group id.
- * @param listener Replica listener.
- * @param raftClient Topology aware Raft client.
+ * @param newConfiguration TODO
+ * @param createListener TODO
* @param storageIndexTracker Storage index tracker.
*/
private CompletableFuture startReplicaInternal(
ReplicationGroupId replicaGrpId,
-ReplicaListener listener,
-TopologyAwareRaftGroupService raftClient,
+PeersAndLearners newConfiguration,
+Function createListener,
PendingComparableValuesTracker storageIndexTracker
-) {
+) throws NodeStoppingException {
LOG.info("Replica is about to start [replicationGroupId={}].",
replicaGrpId);
ClusterNode localNode = clusterNetSvc.topologyService().localMember();
-Replica newReplica = new Replica(
-replicaGrpId,
-listener,
-storageIndexTracker,
-raftClient,
-localNode,
-executor,
-placementDriver,
-clockService
-);
+CompletableFuture newReplicaFut = raftManager
+// TODO IGNITE-19614 This procedure takes 10 seconds if
there's no majority online.
+.startRaftGroupService(replicaGrpId, newConfiguration,
raftGroupServiceFactory, raftCommandsMarshaller)
+.thenApply(createListener)
Review Comment:
let's name it so it could reflect the function entity of this param
--
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
Re: [PR] IGNITE-19681 Java thin: Add tx partition awareness (lazy tx start) [ignite-3]
ptupitsyn merged PR #3640: URL: https://github.com/apache/ignite-3/pull/3640 -- 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
[PR] IGNITE-22024 Fixed ItSqlClientSynchronousApiTest#runtimeErrorInDmlCausesTransactionToFail [ignite-3]
denis-chudov opened a new pull request, #3651: URL: https://github.com/apache/ignite-3/pull/3651 https://issues.apache.org/jira/browse/IGNITE-22024 -- 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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1576029691
##
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogTestUtilsTest.java:
##
@@ -80,7 +80,7 @@ void testManagerWorksAsExpected() throws Exception {
assertThat(tablesOfVersion2, hasItem(descriptorWithName("T1")));
assertThat(tablesOfVersion2, hasItem(descriptorWithName("T2")));
-manager.stop();
+manager.stopAsync();
Review Comment:
Same as above, won't repeat myself below, please add verification that
corresponding future was completed successfully.
--
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
Re: [PR] IGNITE-19681 Java thin: Add tx partition awareness (lazy tx start) [ignite-3]
ptupitsyn commented on code in PR #3640:
URL: https://github.com/apache/ignite-3/pull/3640#discussion_r1576029209
##
modules/client/src/main/java/org/apache/ignite/internal/client/sql/ClientSql.java:
##
@@ -242,7 +243,17 @@ public CompletableFuture>
executeAsync(
if (transaction != null) {
try {
//noinspection resource
-return
ClientTransaction.get(transaction).channel().serviceAsync(ClientOp.SQL_EXEC,
payloadWriter, payloadReader);
+return ClientLazyTransaction.ensureStarted(transaction, ch,
null)
+.thenCompose(tx ->
tx.channel().serviceAsync(ClientOp.SQL_EXEC, payloadWriter, payloadReader))
+.exceptionally(e -> {
+Throwable ex = unwrapCause(e);
+if (ex instanceof TransactionException) {
+var te = (TransactionException) ex;
+throw new SqlException(te.traceId(),
te.code(), te.getMessage(), te);
Review Comment:
> It's already done on server.
`Transaction is already finished` exception is thrown by the client in this
case:
https://github.com/apache/ignite-3/blob/a70aef41a3dc56da9437acead50e76d17259967f/modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransaction.java#L167
--
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
Re: [PR] IGNITE-19681 Java thin: Add tx partition awareness (lazy tx start) [ignite-3]
ptupitsyn commented on code in PR #3640:
URL: https://github.com/apache/ignite-3/pull/3640#discussion_r1576026972
##
modules/client/src/main/java/org/apache/ignite/internal/client/sql/ClientSql.java:
##
@@ -242,7 +243,17 @@ public CompletableFuture>
executeAsync(
if (transaction != null) {
try {
//noinspection resource
-return
ClientTransaction.get(transaction).channel().serviceAsync(ClientOp.SQL_EXEC,
payloadWriter, payloadReader);
+return ClientLazyTransaction.ensureStarted(transaction, ch,
null)
+.thenCompose(tx ->
tx.channel().serviceAsync(ClientOp.SQL_EXEC, payloadWriter, payloadReader))
+.exceptionally(e -> {
+Throwable ex = unwrapCause(e);
+if (ex instanceof TransactionException) {
+var te = (TransactionException) ex;
+throw new SqlException(te.traceId(),
te.code(), te.getMessage(), te);
Review Comment:
This was added specifically to match the embedded behavior and fix
`ItSqlApiBaseTest#checkTransactionsWithDml` (this test runs against client and
embedded APIs to ensure consistency):
https://github.com/apache/ignite-3/blob/0f9f1df95e0006edd9c0b0595a7c2082f2d5cb29/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java#L249
Server-side mapping:
https://github.com/apache/ignite-3/blob/ee00a7c028ef6c8c1d8cdab0169fdd36deb1c5fe/modules/sql-engine/src/main/java/org/apache/ignite/internal/lang/SqlExceptionMapperUtil.java#L61
--
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
Re: [PR] IGNITE-22071 Async component stop [ignite-3]
sanpwc commented on code in PR #3629:
URL: https://github.com/apache/ignite-3/pull/3629#discussion_r1576010330
##
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java:
##
@@ -191,16 +191,18 @@ public CompletableFuture start() {
updateLog.registerUpdateHandler(new OnUpdateHandlerImpl());
-updateLog.start();
+updateLog.startAsync();
Review Comment:
That's not part of your PR actually, but anyway, we should not ignore
`updateLog.startAsync();` future. Proper async postfix 've made it visible.
##
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java:
##
@@ -191,16 +191,18 @@ public CompletableFuture start() {
updateLog.registerUpdateHandler(new OnUpdateHandlerImpl());
-updateLog.start();
+updateLog.startAsync();
return nullCompletedFuture();
}
@Override
-public void stop() throws Exception {
+public CompletableFuture stopAsync() {
busyLock.block();
versionTracker.close();
-updateLog.stop();
+updateLog.stopAsync();
Review Comment:
Basically same as above, that doesn't look right. Why updateLog.stopAsync()
future is ignored?
##
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerSelfTest.java:
##
@@ -239,9 +239,9 @@ public void testNoInteractionsAfterStop() throws Exception {
CompletableFuture readyFuture = manager.catalogReadyFuture(1);
assertFalse(readyFuture.isDone());
-manager.stop();
+manager.stopAsync();
Review Comment:
Here and in other places, let's add verification that corresponding
startAsync/stopAsync futures were completed successfully. I'm talking about
`assertThat(manager.stopAsync(), willCompleteSuccessfully());` In some places
we may combine components start/stop futures and assert that combined one was
completed successfully - not mandatory action though.
--
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
Re: [PR] IGNITE-19681 Java thin: Add tx partition awareness (lazy tx start) [ignite-3]
ascherbakoff commented on code in PR #3640:
URL: https://github.com/apache/ignite-3/pull/3640#discussion_r1576016843
##
modules/client/src/main/java/org/apache/ignite/internal/client/sql/ClientSql.java:
##
@@ -242,7 +243,17 @@ public CompletableFuture>
executeAsync(
if (transaction != null) {
try {
//noinspection resource
-return
ClientTransaction.get(transaction).channel().serviceAsync(ClientOp.SQL_EXEC,
payloadWriter, payloadReader);
+return ClientLazyTransaction.ensureStarted(transaction, ch,
null)
+.thenCompose(tx ->
tx.channel().serviceAsync(ClientOp.SQL_EXEC, payloadWriter, payloadReader))
+.exceptionally(e -> {
+Throwable ex = unwrapCause(e);
+if (ex instanceof TransactionException) {
+var te = (TransactionException) ex;
+throw new SqlException(te.traceId(),
te.code(), te.getMessage(), te);
Review Comment:
Here you create sql exception with txn error code, which is incorrect.
I don't think any unwapping is needed on client side. It's already done on
server.
Same for line 258
--
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
Re: [PR] IGNITE-19681 Java thin: Add tx partition awareness (lazy tx start) [ignite-3]
ptupitsyn commented on code in PR #3640:
URL: https://github.com/apache/ignite-3/pull/3640#discussion_r1576009930
##
modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientTable.java:
##
@@ -419,15 +435,21 @@ private CompletableFuture doSchemaOutInOpAsync(
CompletableFuture.allOf(schemaFut, partitionsFut)
.thenCompose(v -> {
ClientSchema schema = schemaFut.getNow(null);
-String preferredNodeName = getPreferredNodeName(provider,
partitionsFut.getNow(null), schema);
-
-// Perform the operation.
-return ch.serviceAsync(opCode,
-w -> writer.accept(schema, w),
-r -> readSchemaAndReadData(schema, r, reader,
defaultValue, responseSchemaRequired),
-preferredNodeName,
-retryPolicyOverride,
-expectNotifications);
+String txPreferredNodeName =
getPreferredNodeName(provider, partitionsFut.getNow(null), schema);
+
+return ClientLazyTransaction.ensureStarted(tx, ch,
txPreferredNodeName).thenCompose(unused -> {
Review Comment:
Yes, I had this in mind too, but decided to keep the protocol simple.
Ticket created: https://issues.apache.org/jira/browse/IGNITE-22090
--
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
Re: [PR] IGNITE-19681 Java thin: Add tx partition awareness (lazy tx start) [ignite-3]
ascherbakoff commented on code in PR #3640:
URL: https://github.com/apache/ignite-3/pull/3640#discussion_r1575943518
##
modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientTable.java:
##
@@ -419,15 +435,21 @@ private CompletableFuture doSchemaOutInOpAsync(
CompletableFuture.allOf(schemaFut, partitionsFut)
.thenCompose(v -> {
ClientSchema schema = schemaFut.getNow(null);
-String preferredNodeName = getPreferredNodeName(provider,
partitionsFut.getNow(null), schema);
-
-// Perform the operation.
-return ch.serviceAsync(opCode,
-w -> writer.accept(schema, w),
-r -> readSchemaAndReadData(schema, r, reader,
defaultValue, responseSchemaRequired),
-preferredNodeName,
-retryPolicyOverride,
-expectNotifications);
+String txPreferredNodeName =
getPreferredNodeName(provider, partitionsFut.getNow(null), schema);
+
+return ClientLazyTransaction.ensureStarted(tx, ch,
txPreferredNodeName).thenCompose(unused -> {
Review Comment:
This can be optimized even further.
Currently we still have +1RTT to begin tx request/response here, which may
be sensitive to small transactions.
A transaction should be started on first map request.
For this to work logical client tx id should be assigned on client.
For example, id can consist of local client counter combined with client
unique id assigned by server on handshake.
One bit of 64 bit id is reserved for "first" flag.
If an operation is "first", the txn is implicitly started.
I'm ok to do it in a separate PR.
--
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
Re: [PR] IGNITE-19681 Java thin: Add tx partition awareness (lazy tx start) [ignite-3]
ascherbakoff commented on code in PR #3640:
URL: https://github.com/apache/ignite-3/pull/3640#discussion_r1575943518
##
modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientTable.java:
##
@@ -419,15 +435,21 @@ private CompletableFuture doSchemaOutInOpAsync(
CompletableFuture.allOf(schemaFut, partitionsFut)
.thenCompose(v -> {
ClientSchema schema = schemaFut.getNow(null);
-String preferredNodeName = getPreferredNodeName(provider,
partitionsFut.getNow(null), schema);
-
-// Perform the operation.
-return ch.serviceAsync(opCode,
-w -> writer.accept(schema, w),
-r -> readSchemaAndReadData(schema, r, reader,
defaultValue, responseSchemaRequired),
-preferredNodeName,
-retryPolicyOverride,
-expectNotifications);
+String txPreferredNodeName =
getPreferredNodeName(provider, partitionsFut.getNow(null), schema);
+
+return ClientLazyTransaction.ensureStarted(tx, ch,
txPreferredNodeName).thenCompose(unused -> {
Review Comment:
This can be optimized even further.
Currently we still have +1RTT due to begin tx request/response here, which
may be sensitive to small transactions.
A transaction should be started on first map request.
For this to work logical client tx id should be assigned on client.
For example, id can consist of local client counter combined with client
unique id assigned by server on handshake.
One bit of 64 bit id is reserved for "first" flag.
If an operation is "first", the txn is implicitly started.
I'm ok to do it in a separate PR.
--
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
Re: [PR] IGNITE-21943: Cover SQL F561(Full value expressions) feature by tests [ignite-3]
ygerzhedovich merged PR #3641: URL: https://github.com/apache/ignite-3/pull/3641 -- 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
Re: [PR] IGNITE-21763 Adjust TxnResourceVacuumTask in order to vacuum persistent txn state [ignite-3]
denis-chudov commented on code in PR #3591:
URL: https://github.com/apache/ignite-3/pull/3591#discussion_r1575792698
##
modules/transactions/src/testFixtures/java/org/apache/ignite/internal/tx/storage/state/test/TestTxStateStorage.java:
##
@@ -108,10 +108,15 @@ public boolean compareAndSet(UUID txId, @Nullable TxState
txStateExpected, TxMet
}
@Override
-public void remove(UUID txId) {
+public void remove(UUID txId, long commandIndex, long commandTerm) {
checkStorageClosedOrInProgressOfRebalance();
storage.remove(txId);
+
+if (rebalanceFutureReference.get() == null) {
+lastAppliedIndex = commandIndex;
Review Comment:
in any case, the `put` issue is not related to this ticket, let's create a
separate one
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
