sanpwc commented on code in PR #946: URL: https://github.com/apache/ignite-3/pull/946#discussion_r924383634
########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxMeta.java: ########## @@ -0,0 +1,65 @@ +/* + * Copyright 2022 GridGain Systems, Inc. and Contributors. + * + * Licensed under the GridGain Community Edition License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license + * + * 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.tx; + +import java.io.Serializable; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import org.apache.ignite.lang.IgniteBiTuple; + +import static java.util.Collections.unmodifiableList; + +public class TxMeta implements Serializable { + private final TxState txState; + + private final List<IgniteBiTuple<Integer, Integer>> enlistedPartitions; Review Comment: I'd rather use replicationGroupId (paritionId), meaning String. ########## modules/core/src/main/java/org/apache/ignite/lang/ErrorGroups.java: ########## @@ -44,4 +44,19 @@ public static class Table { /** Column not found. */ public static int COLUMN_NOT_FOUND_ERR = TABLE_ERR_GROUP.registerErrorCode(4); } + + /** Transactions error group. */ + public static class Transactions { + /** Transactions error group. */ + public static final ErrorGroup TX_ERR_GROUP = ErrorGroup.newGroup("TX", 3); + + /** Error on creation of table state storage. */ Review Comment: Probably it's transaction state storage instead of **table** state storage. ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/TxMetaStorage.java: ########## @@ -0,0 +1,96 @@ +/* + * Copyright 2022 GridGain Systems, Inc. and Contributors. + * + * Licensed under the GridGain Community Edition License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license + * + * 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.tx.storage.state; + +import java.nio.file.Path; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.tx.TxMeta; + +/** + * Storage for transaction meta, {@link TxMeta}. + */ +public interface TxMetaStorage extends AutoCloseable { Review Comment: Confusing naming. ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxMeta.java: ########## @@ -0,0 +1,65 @@ +/* + * Copyright 2022 GridGain Systems, Inc. and Contributors. + * + * Licensed under the GridGain Community Edition License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license + * + * 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.tx; + +import java.io.Serializable; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import org.apache.ignite.lang.IgniteBiTuple; + +import static java.util.Collections.unmodifiableList; + +public class TxMeta implements Serializable { + private final TxState txState; + + private final List<IgniteBiTuple<Integer, Integer>> enlistedPartitions; + + private final long commitTimestamp; Review Comment: I believe that HybridTimestamp suites better here. ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/TxMetaStorage.java: ########## @@ -0,0 +1,96 @@ +/* + * Copyright 2022 GridGain Systems, Inc. and Contributors. + * + * Licensed under the GridGain Community Edition License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license + * + * 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.tx.storage.state; + +import java.nio.file.Path; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.tx.TxMeta; + +/** + * Storage for transaction meta, {@link TxMeta}. + */ +public interface TxMetaStorage extends AutoCloseable { + /** + * Start the storage. + */ + void start(); + + /** + * Whether the storage is started. + * + * @return {@code true} if the storage is started, {@code false} otherwise. + */ + boolean isStarted(); + + /** + * Stop the storage. + */ + void stop() throws Exception; + + /** + * Get tx meta by tx id. + * + * @param txId Tx id. + * @return Tx meta. + */ + TxMeta get(UUID txId); Review Comment: Here and there: let's declare possible exceptions within javadoc. ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/TxMetaStorage.java: ########## @@ -0,0 +1,96 @@ +/* + * Copyright 2022 GridGain Systems, Inc. and Contributors. + * + * Licensed under the GridGain Community Edition License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license + * + * 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.tx.storage.state; + +import java.nio.file.Path; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.tx.TxMeta; + +/** + * Storage for transaction meta, {@link TxMeta}. + */ +public interface TxMetaStorage extends AutoCloseable { + /** + * Start the storage. + */ + void start(); + + /** + * Whether the storage is started. + * + * @return {@code true} if the storage is started, {@code false} otherwise. + */ + boolean isStarted(); + + /** + * Stop the storage. + */ + void stop() throws Exception; + + /** + * Get tx meta by tx id. + * + * @param txId Tx id. + * @return Tx meta. + */ + TxMeta get(UUID txId); + + /** + * Put the tx meta into the storage. + * + * @param txId Tx id. + * @param txMeta Tx meta. + */ + void put(UUID txId, TxMeta txMeta); + + /** + * Atomically change the tx meta in the storage. + * + * @param txId Tx id. + * @param txMetaExpected Tx meta that is expected to be in the storage. + * @param txMeta Tx meta. + * @return Whether the CAS operation is successful. + */ + boolean compareAndSet(UUID txId, TxMeta txMetaExpected, TxMeta txMeta); Review Comment: Not sure whether we will need such method, if we will, it'll be probably `boolean compareAndSet(UUID txId, TxState txStateExpected, TxMeta txMeta);` I'd rather remove given method for now. Up to you. ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxMeta.java: ########## @@ -0,0 +1,65 @@ +/* + * Copyright 2022 GridGain Systems, Inc. and Contributors. + * + * Licensed under the GridGain Community Edition License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license + * + * 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.tx; + +import java.io.Serializable; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import org.apache.ignite.lang.IgniteBiTuple; + +import static java.util.Collections.unmodifiableList; + +public class TxMeta implements Serializable { + private final TxState txState; + + private final List<IgniteBiTuple<Integer, Integer>> enlistedPartitions; + + private final long commitTimestamp; + + public TxMeta(TxState txState, List<IgniteBiTuple<Integer, Integer>> enlistedPartitions, long commitTimestamp) { + this.txState = txState; + this.enlistedPartitions = enlistedPartitions; + this.commitTimestamp = commitTimestamp; + } + + public TxState txState() { + return txState; + } + + public List<IgniteBiTuple<Integer, Integer>> enlistedPartitions() { + return unmodifiableList(enlistedPartitions); + } + + public long commitTimestamp() { + return commitTimestamp; + } + + @Override public boolean equals(Object o) { Review Comment: Do we really need equals and hashCode for meta? Why? ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/inmemory/TxMetaRowWrapper.java: ########## @@ -0,0 +1,60 @@ +/* + * Copyright 2022 GridGain Systems, Inc. and Contributors. + * + * Licensed under the GridGain Community Edition License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license + * + * 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.tx.storage.state.inmemory; Review Comment: Why it's inmemory? It's probably pagememory. ########## modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/inmemory/TxMetaVolatileInMemoryStorage.java: ########## @@ -0,0 +1,138 @@ +/* + * Copyright 2022 GridGain Systems, Inc. and Contributors. + * + * Licensed under the GridGain Community Edition License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license + * + * 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.tx.storage.state.inmemory; + +import java.nio.file.Path; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import org.apache.ignite.internal.pagememory.tree.IgniteTree.InvokeClosure; +import org.apache.ignite.internal.pagememory.tree.IgniteTree.OperationType; +import org.apache.ignite.internal.tx.TxMeta; +import org.apache.ignite.internal.tx.storage.state.TxMetaStorage; +import org.apache.ignite.lang.IgniteInternalCheckedException; +import org.apache.ignite.lang.IgniteInternalException; +import org.jetbrains.annotations.Nullable; + +import static org.apache.ignite.internal.tx.storage.state.inmemory.TxMetaRowWrapper.unwrap; +import static org.apache.ignite.lang.ErrorGroups.Transactions.TX_STATE_STORAGE_DESTROY_ERR; +import static org.apache.ignite.lang.ErrorGroups.Transactions.TX_STATE_STORAGE_ERR; + +public class TxMetaVolatileInMemoryStorage implements TxMetaStorage { Review Comment: I'm not sure whether it's a valid case to have im-memory txnState. Probably it is, but without extra effort within writeIntent resolution we may read inconsistent data. Let's discuss it explicitly. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
