diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml index 97895d53fea..7f5fd9303ee 100644 --- a/.palantir/revapi.yml +++ b/.palantir/revapi.yml @@ -1,5 +1,17 @@ acceptedBreaks: "0.1040.0": + com.palantir.atlasdb:atlasdb-api: + - code: "java.method.removed" + old: "method java.util.Map\ + \ com.palantir.atlasdb.cell.api.AutoDelegate_TransactionKeyValueService::get(com.palantir.atlasdb.keyvalue.api.TableReference,\ + \ java.util.Map)" + justification: "TransactionKeyValueService API not widely used yet" + - code: "java.method.removed" + old: "method java.util.Map\ + \ com.palantir.atlasdb.cell.api.TransactionKeyValueService::get(com.palantir.atlasdb.keyvalue.api.TableReference,\ + \ java.util.Map)" + justification: "TransactionKeyValueService API not widely used yet" + "0.1041.0-rc3": com.palantir.atlasdb:atlasdb-api: - code: "java.class.removed" old: "interface com.palantir.atlasdb.transaction.api.AutoDelegate_TransactionKeyValueServiceManager" @@ -7,6 +19,47 @@ acceptedBreaks: - code: "java.class.removed" old: "interface com.palantir.atlasdb.transaction.api.TransactionKeyValueServiceManager" justification: "Still iterating on the APIs" + - code: "java.method.addedToInterface" + new: "method java.util.NavigableMap>\ + \ com.palantir.atlasdb.transaction.api.KeyValueSnapshotReader::getRows(com.palantir.atlasdb.keyvalue.api.TableReference,\ + \ java.lang.Iterable, com.palantir.atlasdb.keyvalue.api.ColumnSelection,\ + \ com.google.common.collect.ImmutableMap.Builder)" + justification: "SnapshotReader and TimestampLoader are new internal interfaces" + - code: "java.method.addedToInterface" + new: "method void com.palantir.atlasdb.transaction.api.KeyValueSnapshotEventRecorder::recordEmptyValueRead(com.palantir.atlasdb.keyvalue.api.TableReference)" + justification: "SnapshotReader and TimestampLoader are new internal interfaces" + - code: "java.method.numberOfParametersChanged" + old: "method com.google.common.util.concurrent.ListenableFuture\ + \ com.palantir.atlasdb.transaction.api.CommitTimestampLoader::getCommitTimestamps(com.palantir.atlasdb.keyvalue.api.TableReference,\ + \ org.eclipse.collections.api.LongIterable, boolean, com.palantir.atlasdb.transaction.service.AsyncTransactionService)" + new: "method com.google.common.util.concurrent.ListenableFuture\ + \ com.palantir.atlasdb.transaction.api.CommitTimestampLoader::getCommitTimestamps(com.palantir.atlasdb.keyvalue.api.TableReference,\ + \ org.eclipse.collections.api.LongIterable, boolean)" + justification: "SnapshotReader and TimestampLoader are new internal interfaces" + - code: "java.method.numberOfParametersChanged" + old: "method com.palantir.atlasdb.transaction.api.TransactionKeyValueServiceManager\ + \ com.palantir.atlasdb.spi.TransactionKeyValueServiceManagerFactory::create(com.palantir.atlasdb.coordination.CoordinationService,\ + \ com.palantir.atlasdb.spi.KeyValueServiceManager, com.palantir.atlasdb.spi.TransactionKeyValueServiceConfig,\ + \ com.palantir.refreshable.Refreshable,\ + \ boolean)" + new: "method com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager\ + \ com.palantir.atlasdb.spi.TransactionKeyValueServiceManagerFactory::create(java.lang.String,\ + \ com.palantir.dialogue.clients.DialogueClients.ReloadingFactory, com.palantir.atlasdb.util.MetricsManager,\ + \ com.palantir.atlasdb.coordination.CoordinationService, com.palantir.atlasdb.spi.KeyValueServiceManager,\ + \ com.palantir.atlasdb.spi.TransactionKeyValueServiceConfig, com.palantir.refreshable.Refreshable,\ + \ boolean)" + justification: "updating internal APIs" + - code: "java.method.parameterTypeChanged" + old: "parameter com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager\ + \ com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManagerFactory::createKeyValueSnapshotReaderManager(===com.palantir.atlasdb.transaction.api.TransactionKeyValueServiceManager===,\ + \ com.palantir.atlasdb.transaction.service.TransactionService, boolean, com.palantir.atlasdb.transaction.api.OrphanedSentinelDeleter,\ + \ com.palantir.atlasdb.transaction.api.DeleteExecutor)" + new: "parameter com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager\ + \ com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManagerFactory::createKeyValueSnapshotReaderManager(===com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager===,\ + \ com.palantir.atlasdb.transaction.service.TransactionService, boolean, com.palantir.atlasdb.transaction.api.OrphanedSentinelDeleter,\ + \ com.palantir.atlasdb.transaction.api.DeleteExecutor)" + justification: "updating internal APIs" - code: "java.method.returnTypeChanged" old: "method com.palantir.atlasdb.transaction.api.TransactionKeyValueServiceManager\ \ com.palantir.atlasdb.spi.TransactionKeyValueServiceManagerFactory::create(com.palantir.atlasdb.coordination.CoordinationService,\ @@ -19,6 +72,19 @@ acceptedBreaks: \ com.palantir.refreshable.Refreshable,\ \ boolean)" justification: "Still iterating on the APIs" + - code: "java.method.returnTypeChanged" + old: "method com.palantir.atlasdb.transaction.api.TransactionKeyValueServiceManager\ + \ com.palantir.atlasdb.spi.TransactionKeyValueServiceManagerFactory::create(com.palantir.atlasdb.coordination.CoordinationService,\ + \ com.palantir.atlasdb.spi.KeyValueServiceManager, com.palantir.atlasdb.spi.TransactionKeyValueServiceConfig,\ + \ com.palantir.refreshable.Refreshable,\ + \ boolean)" + new: "method com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager\ + \ com.palantir.atlasdb.spi.TransactionKeyValueServiceManagerFactory::create(java.lang.String,\ + \ com.palantir.dialogue.clients.DialogueClients.ReloadingFactory, com.palantir.atlasdb.util.MetricsManager,\ + \ com.palantir.atlasdb.coordination.CoordinationService, com.palantir.atlasdb.spi.KeyValueServiceManager,\ + \ com.palantir.atlasdb.spi.TransactionKeyValueServiceConfig, com.palantir.refreshable.Refreshable,\ + \ boolean)" + justification: "updating internal APIs" com.palantir.atlasdb:atlasdb-config: - code: "java.method.numberOfParametersChanged" old: "method com.palantir.atlasdb.factory.TransactionManagersInitializer com.palantir.atlasdb.factory.TransactionManagersInitializer::createInitialTables(com.palantir.atlasdb.keyvalue.api.KeyValueService,\ @@ -47,18 +113,18 @@ acceptedBreaks: - code: "java.method.addedToInterface" new: "method boolean com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager::isInitialized()" justification: "Adding async init to TransactionKeyValueServiceManager" - "0.1050.0": + "0.1051.0-rc1": com.palantir.atlasdb:atlasdb-api: - - code: "java.method.removed" - old: "method java.util.Map\ - \ com.palantir.atlasdb.cell.api.AutoDelegate_TransactionKeyValueService::get(com.palantir.atlasdb.keyvalue.api.TableReference,\ - \ java.util.Map)" - justification: "TransactionKeyValueService is largely internal" - - code: "java.method.removed" - old: "method java.util.Map\ - \ com.palantir.atlasdb.cell.api.TransactionKeyValueService::get(com.palantir.atlasdb.keyvalue.api.TableReference,\ - \ java.util.Map)" - justification: "TransactionKeyValueService is largely internal" + - code: "java.method.numberOfParametersChanged" + old: "method com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager\ + \ com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManagerFactory::createKeyValueSnapshotReaderManager(com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager,\ + \ com.palantir.atlasdb.transaction.service.TransactionService, boolean, com.palantir.atlasdb.transaction.api.OrphanedSentinelDeleter,\ + \ com.palantir.atlasdb.transaction.api.DeleteExecutor)" + new: "method com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager\ + \ com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManagerFactory::createKeyValueSnapshotReaderManager(com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager,\ + \ com.palantir.atlasdb.transaction.service.TransactionService, boolean, com.palantir.atlasdb.transaction.api.OrphanedSentinelDeleter,\ + \ com.palantir.atlasdb.transaction.api.DeleteExecutor, com.palantir.atlasdb.util.MetricsManager)" + justification: "internal API" "0.770.0": com.palantir.atlasdb:atlasdb-api: - code: "java.class.removed" diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DeleteExecutor.java b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/DeleteExecutor.java similarity index 95% rename from atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DeleteExecutor.java rename to atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/DeleteExecutor.java index acacee8021c..2e4e7b7e5b0 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DeleteExecutor.java +++ b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/DeleteExecutor.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.palantir.atlasdb.transaction.impl; +package com.palantir.atlasdb.transaction.api; import com.palantir.atlasdb.keyvalue.api.Cell; import com.palantir.atlasdb.keyvalue.api.TableReference; diff --git a/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotEventRecorder.java b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotEventRecorder.java new file mode 100644 index 00000000000..6c5d18c7798 --- /dev/null +++ b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotEventRecorder.java @@ -0,0 +1,37 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.api; + +import com.palantir.atlasdb.keyvalue.api.TableReference; + +public interface KeyValueSnapshotEventRecorder { + void recordCellsRead(TableReference tableReference, long cellsRead); + + void recordCellsReturned(TableReference tableReference, long cellsReturned); + + void recordManyBytesReadForTable(TableReference tableReference, long bytesRead); + + void recordFilteredSweepSentinel(TableReference tableReference); + + void recordFilteredUncommittedTransaction(TableReference tableReference); + + void recordFilteredTransactionCommittingAfterOurStart(TableReference tableReference); + + void recordRolledBackOtherTransaction(); + + void recordEmptyValueRead(TableReference tableReference); +} diff --git a/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotReader.java b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotReader.java new file mode 100644 index 00000000000..53250d2f828 --- /dev/null +++ b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotReader.java @@ -0,0 +1,52 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.api; + +import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.ListenableFuture; +import com.palantir.atlasdb.keyvalue.api.Cell; +import com.palantir.atlasdb.keyvalue.api.ColumnSelection; +import com.palantir.atlasdb.keyvalue.api.RowResult; +import com.palantir.atlasdb.keyvalue.api.TableReference; +import java.util.Map; +import java.util.NavigableMap; +import java.util.Set; + +/** + * Reads state of a {@link com.palantir.atlasdb.keyvalue.api.KeyValueService} in accordance with the provided + * AtlasDB timestamp, following the AtlasDB read protocol. This includes reading the most recent committed value for + * each cell that would be visible at the provided timestamp(s) and filtering out versions that have been aborted or + * not committed yet. + *

+ * If used in the context of a transaction, users are responsible for validating that snapshots read are still + * guaranteed to be consistent (for example, transactions may need to validate their pre-commit conditions or check + * that sweep has not progressed). Some methods may have partial, intermediate validation required as part of servicing + * a read; this class will carry out this intermediate validation. + *

+ * Although this interface performs user-level reads, internal writes may be performed (for example, as part of the + * read protocol, to abort a long-running transaction). + */ +public interface KeyValueSnapshotReader { + ListenableFuture> getAsync(TableReference tableReference, Set cells); + + // TODO (jkong): This one is kind of awful, but it's used to avoid allocations and I'm not keen to add too many. + NavigableMap> getRows( + TableReference tableReference, + Iterable rows, + ColumnSelection columnSelection, + ImmutableMap.Builder resultCollector); +} diff --git a/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotReaderManager.java b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotReaderManager.java new file mode 100644 index 00000000000..b906fa9d266 --- /dev/null +++ b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotReaderManager.java @@ -0,0 +1,37 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.api; + +import java.util.function.LongSupplier; +import org.immutables.value.Value; + +public interface KeyValueSnapshotReaderManager { + KeyValueSnapshotReader createKeyValueSnapshotReader(TransactionContext transactionContext); + + @Value.Immutable + interface TransactionContext { + LongSupplier startTimestampSupplier(); + + TransactionReadSentinelBehavior transactionReadSentinelBehavior(); + + CommitTimestampLoader commitTimestampLoader(); + + PreCommitRequirementValidator preCommitRequirementValidator(); + + KeyValueSnapshotEventRecorder keyValueSnapshotEventRecorder(); + } +} diff --git a/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotReaderManagerFactory.java b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotReaderManagerFactory.java new file mode 100644 index 00000000000..6dee057400b --- /dev/null +++ b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/KeyValueSnapshotReaderManagerFactory.java @@ -0,0 +1,33 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.api; + +import com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager; +import com.palantir.atlasdb.transaction.service.TransactionService; +import com.palantir.atlasdb.util.MetricsManager; + +public interface KeyValueSnapshotReaderManagerFactory { + String getType(); + + KeyValueSnapshotReaderManager createKeyValueSnapshotReaderManager( + TransactionKeyValueServiceManager transactionKeyValueServiceManager, + TransactionService transactionService, + boolean allowHiddenTableAccess, + OrphanedSentinelDeleter orphanedSentinelDeleter, + DeleteExecutor deleteExecutor, + MetricsManager metricsManager); +} diff --git a/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/OrphanedSentinelDeleter.java b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/OrphanedSentinelDeleter.java new file mode 100644 index 00000000000..52cee22e72f --- /dev/null +++ b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/OrphanedSentinelDeleter.java @@ -0,0 +1,25 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.api; + +import com.palantir.atlasdb.keyvalue.api.Cell; +import com.palantir.atlasdb.keyvalue.api.TableReference; +import java.util.Set; + +public interface OrphanedSentinelDeleter { + void scheduleSentinelsForDeletion(TableReference tableReference, Set orphanedSentinels); +} diff --git a/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/PreCommitRequirementValidator.java b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/PreCommitRequirementValidator.java new file mode 100644 index 00000000000..94e6b6d4473 --- /dev/null +++ b/atlasdb-api/src/main/java/com/palantir/atlasdb/transaction/api/PreCommitRequirementValidator.java @@ -0,0 +1,38 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.api; + +import com.palantir.atlasdb.keyvalue.api.Cell; +import com.palantir.atlasdb.keyvalue.api.TableReference; +import com.palantir.lock.v2.LockToken; +import java.util.Map; +import java.util.Optional; + +public interface PreCommitRequirementValidator { + // TODO (jkong): The boolean means "are there unvalidated reads"? + boolean throwIfPreCommitRequirementsNotMetOnRead( + TableReference tableRef, long timestamp, boolean allPossibleCellsReadAndPresent); + + void throwIfPreCommitConditionInvalid(long timestamp); + + void throwIfPreCommitConditionInvalidAtCommitOnWriteTransaction( + Map> mutations, long timestamp); + + void throwIfPreCommitRequirementsNotMet(Optional commitLocksToken, long timestamp); + + void throwIfImmutableTsOrCommitLocksExpired(Optional commitLocksToken); +} diff --git a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/AtlasDbServiceDiscovery.java b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/AtlasDbServiceDiscovery.java index 8552886ba62..33b64d258fc 100644 --- a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/AtlasDbServiceDiscovery.java +++ b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/AtlasDbServiceDiscovery.java @@ -22,6 +22,7 @@ import com.palantir.atlasdb.spi.TransactionKeyValueServiceConfig; import com.palantir.atlasdb.spi.TransactionKeyValueServiceManagerFactory; import com.palantir.atlasdb.timestamp.DbTimeLockFactory; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManagerFactory; import com.palantir.logsafe.SafeArg; import com.palantir.logsafe.exceptions.SafeIllegalStateException; import java.util.ServiceLoader; @@ -44,6 +45,15 @@ public static AtlasDbFactory createAtlasFactoryOfCorrectType(KeyValueServiceConf TransactionKeyValueServiceManagerFactory.class); } + // TODO (jkong): A little cheaty, currently coupling usage of TKVSMF and KVSRF to be special or non-special... + public static KeyValueSnapshotReaderManagerFactory createKeyValueSnapshotReaderManagerFactoryOfCorrectType( + TransactionKeyValueServiceConfig config) { + return createAtlasDbServiceOfCorrectType( + config.type(), + KeyValueSnapshotReaderManagerFactory::getType, + KeyValueSnapshotReaderManagerFactory.class); + } + public static DbTimeLockFactory createDbTimeLockFactoryOfCorrectType(KeyValueServiceConfig config) { return createAtlasDbServiceOfCorrectType(config, DbTimeLockFactory::getType, DbTimeLockFactory.class); } diff --git a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/TransactionManagers.java b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/TransactionManagers.java index 86a60317867..83b48edf24c 100644 --- a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/TransactionManagers.java +++ b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/TransactionManagers.java @@ -95,11 +95,18 @@ import com.palantir.atlasdb.transaction.ImmutableTransactionConfig; import com.palantir.atlasdb.transaction.TransactionConfig; import com.palantir.atlasdb.transaction.api.AtlasDbConstraintCheckingMode; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManagerFactory; import com.palantir.atlasdb.transaction.api.LockWatchingCache; import com.palantir.atlasdb.transaction.api.NoOpLockWatchingCache; +import com.palantir.atlasdb.transaction.api.OrphanedSentinelDeleter; import com.palantir.atlasdb.transaction.api.TransactionManager; import com.palantir.atlasdb.transaction.impl.ConflictDetectionManager; import com.palantir.atlasdb.transaction.impl.ConflictDetectionManagers; +import com.palantir.atlasdb.transaction.impl.DefaultDeleteExecutor; +import com.palantir.atlasdb.transaction.impl.DefaultKeyValueSnapshotReaderManager; +import com.palantir.atlasdb.transaction.impl.DefaultOrphanedSentinelDeleter; import com.palantir.atlasdb.transaction.impl.SerializableTransactionManager; import com.palantir.atlasdb.transaction.impl.SweepStrategyManager; import com.palantir.atlasdb.transaction.impl.SweepStrategyManagers; @@ -541,6 +548,15 @@ private TransactionManager serializableInternal(@Output List clos asyncInitializationCallback(), createClearsTable(internalKeyValueService))); + // TODO (jkong): Allow user to inject here + DeleteExecutor deleteExecutor = DefaultDeleteExecutor.createDefault(internalKeyValueService); + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager = createKeyValueSnapshotReaderManager( + transactionKeyValueServiceManager, + transactionService, + sweepStrategyManager, + deleteExecutor, + metricsManager); + TransactionManager transactionManager = initializeCloseable( () -> SerializableTransactionManager.createInstrumented( metricsManager, @@ -568,7 +584,9 @@ private TransactionManager serializableInternal(@Output List clos conflictTracer, metricsFilterEvaluationContext(), installConfig.sharedResourcesConfig().map(SharedResourcesConfig::sharedGetRangesPoolSize), - knowledge), + knowledge, + deleteExecutor, + keyValueSnapshotReaderManager), closeables); transactionManager.registerClosingCallback(runtimeConfigRefreshable::close); @@ -606,6 +624,32 @@ private TransactionManager serializableInternal(@Output List clos return transactionManager; } + private KeyValueSnapshotReaderManager createKeyValueSnapshotReaderManager( + TransactionKeyValueServiceManager transactionKeyValueServiceManager, + TransactionService transactionService, + SweepStrategyManager sweepStrategyManager, + DeleteExecutor deleteExecutor, + MetricsManager metricsManager) { + Optional serviceDiscoveredFactory = config().transactionKeyValueService() + .map(AtlasDbServiceDiscovery::createKeyValueSnapshotReaderManagerFactoryOfCorrectType); + OrphanedSentinelDeleter orphanedSentinelDeleter = + new DefaultOrphanedSentinelDeleter(sweepStrategyManager::get, deleteExecutor); + return serviceDiscoveredFactory + .map(factory -> factory.createKeyValueSnapshotReaderManager( + transactionKeyValueServiceManager, + transactionService, + allowHiddenTableAccess(), + orphanedSentinelDeleter, + deleteExecutor, + metricsManager)) + .orElseGet(() -> new DefaultKeyValueSnapshotReaderManager( + transactionKeyValueServiceManager, + transactionService, + allowHiddenTableAccess(), + orphanedSentinelDeleter, + deleteExecutor)); + } + private TransactionKeyValueServiceManager createTransactionKeyValueServiceManager( TransactionKeyValueServiceManagerFactory factory, MetricsManager metricsManager, diff --git a/atlasdb-config/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable b/atlasdb-config/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable index f3075f35c61..4a3a22df06a 100644 --- a/atlasdb-config/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable +++ b/atlasdb-config/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable @@ -1,2 +1,4 @@ com.palantir.atlasdb.spi.KeyValueServiceConfig com.palantir.atlasdb.spi.KeyValueServiceRuntimeConfig +com.palantir.atlasdb.spi.TransactionKeyValueServiceConfig +com.palantir.atlasdb.spi.TransactionKeyValueServiceRuntimeConfig diff --git a/atlasdb-dagger/src/main/java/com/palantir/atlasdb/services/TransactionManagerModule.java b/atlasdb-dagger/src/main/java/com/palantir/atlasdb/services/TransactionManagerModule.java index e1225daf959..3dd64b11110 100644 --- a/atlasdb-dagger/src/main/java/com/palantir/atlasdb/services/TransactionManagerModule.java +++ b/atlasdb-dagger/src/main/java/com/palantir/atlasdb/services/TransactionManagerModule.java @@ -18,6 +18,7 @@ import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.palantir.atlasdb.cache.DefaultTimestampCache; +import com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager; import com.palantir.atlasdb.cleaner.CleanupFollower; import com.palantir.atlasdb.cleaner.DefaultCleanerBuilder; import com.palantir.atlasdb.cleaner.Follower; @@ -37,6 +38,8 @@ import com.palantir.atlasdb.transaction.api.AtlasDbConstraintCheckingMode; import com.palantir.atlasdb.transaction.impl.ConflictDetectionManager; import com.palantir.atlasdb.transaction.impl.DefaultDeleteExecutor; +import com.palantir.atlasdb.transaction.impl.DefaultKeyValueSnapshotReaderManager; +import com.palantir.atlasdb.transaction.impl.DefaultOrphanedSentinelDeleter; import com.palantir.atlasdb.transaction.impl.SerializableTransactionManager; import com.palantir.atlasdb.transaction.impl.SweepStrategyManager; import com.palantir.atlasdb.transaction.impl.metrics.DefaultMetricsFilterEvaluationContext; @@ -120,9 +123,15 @@ public SerializableTransactionManager provideTransactionManager( @Internal DerivedSnapshotConfig derivedSnapshotConfig, TransactionKnowledgeComponents knowledge) { // todo(gmaretic): should this be using a real sweep queue? + TransactionKeyValueServiceManager transactionKeyValueServiceManager = + new DelegatingTransactionKeyValueServiceManager(kvs); + DefaultDeleteExecutor deleteExecutor = new DefaultDeleteExecutor( + kvs, + Executors.newSingleThreadExecutor( + new NamedThreadFactory(TransactionManagerModule.class + "-delete-executor", true))); return new SerializableTransactionManager( metricsManager, - new DelegatingTransactionKeyValueServiceManager(kvs), + transactionKeyValueServiceManager, lts.timelock(), lts.lockWatcher(), lts.managedTimestampService(), @@ -138,15 +147,18 @@ public SerializableTransactionManager provideTransactionManager( derivedSnapshotConfig.concurrentGetRangesThreadPoolSize(), derivedSnapshotConfig.defaultGetRangesConcurrency(), MultiTableSweepQueueWriter.NO_OP, - new DefaultDeleteExecutor( - kvs, - Executors.newSingleThreadExecutor( - new NamedThreadFactory(TransactionManagerModule.class + "-delete-executor", true))), + deleteExecutor, true, () -> config.atlasDbRuntimeConfig().transaction(), ConflictTracer.NO_OP, DefaultMetricsFilterEvaluationContext.createDefault(), Optional.empty(), - knowledge); + knowledge, + new DefaultKeyValueSnapshotReaderManager( + transactionKeyValueServiceManager, + transactionService, + config.allowAccessToHiddenTables(), + new DefaultOrphanedSentinelDeleter(sweepStrategyManager::get, deleteExecutor), + deleteExecutor)); } } diff --git a/atlasdb-dagger/src/main/java/com/palantir/atlasdb/services/test/TestTransactionManagerModule.java b/atlasdb-dagger/src/main/java/com/palantir/atlasdb/services/test/TestTransactionManagerModule.java index 1cc02ff82e5..88db51cf0c9 100644 --- a/atlasdb-dagger/src/main/java/com/palantir/atlasdb/services/test/TestTransactionManagerModule.java +++ b/atlasdb-dagger/src/main/java/com/palantir/atlasdb/services/test/TestTransactionManagerModule.java @@ -18,6 +18,7 @@ import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.palantir.atlasdb.cache.DefaultTimestampCache; +import com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager; import com.palantir.atlasdb.cleaner.CleanupFollower; import com.palantir.atlasdb.cleaner.DefaultCleanerBuilder; import com.palantir.atlasdb.cleaner.Follower; @@ -38,6 +39,8 @@ import com.palantir.atlasdb.transaction.api.AtlasDbConstraintCheckingMode; import com.palantir.atlasdb.transaction.impl.ConflictDetectionManager; import com.palantir.atlasdb.transaction.impl.DefaultDeleteExecutor; +import com.palantir.atlasdb.transaction.impl.DefaultKeyValueSnapshotReaderManager; +import com.palantir.atlasdb.transaction.impl.DefaultOrphanedSentinelDeleter; import com.palantir.atlasdb.transaction.impl.SerializableTransactionManager; import com.palantir.atlasdb.transaction.impl.SweepStrategyManager; import com.palantir.atlasdb.transaction.impl.metrics.DefaultMetricsFilterEvaluationContext; @@ -120,6 +123,10 @@ public SerializableTransactionManager provideTransactionManager( Cleaner cleaner, @Internal DerivedSnapshotConfig derivedSnapshotConfig, TransactionKnowledgeComponents knowledge) { + TransactionKeyValueServiceManager transactionKeyValueServiceManager = + new DelegatingTransactionKeyValueServiceManager(kvs); + DefaultDeleteExecutor deleteExecutor = + new DefaultDeleteExecutor(kvs, PTExecutors.newSingleThreadExecutor(true)); return new SerializableTransactionManager( metricsManager, new DelegatingTransactionKeyValueServiceManager(kvs), @@ -138,12 +145,18 @@ public SerializableTransactionManager provideTransactionManager( derivedSnapshotConfig.concurrentGetRangesThreadPoolSize(), derivedSnapshotConfig.defaultGetRangesConcurrency(), MultiTableSweepQueueWriter.NO_OP, - new DefaultDeleteExecutor(kvs, PTExecutors.newSingleThreadExecutor(true)), + deleteExecutor, true, () -> config.atlasDbRuntimeConfig().transaction(), ConflictTracer.NO_OP, DefaultMetricsFilterEvaluationContext.createDefault(), Optional.empty(), - knowledge); + knowledge, + new DefaultKeyValueSnapshotReaderManager( + transactionKeyValueServiceManager, + transactionService, + config.allowAccessToHiddenTables(), + new DefaultOrphanedSentinelDeleter(sweepStrategyManager::get, deleteExecutor), + deleteExecutor)); } } diff --git a/atlasdb-impl-shared/build.gradle b/atlasdb-impl-shared/build.gradle index 03b75c4312d..9cc9920f2da 100644 --- a/atlasdb-impl-shared/build.gradle +++ b/atlasdb-impl-shared/build.gradle @@ -45,6 +45,7 @@ dependencies { implementation 'com.palantir.tritium:tritium-registry' implementation 'io.dropwizard.metrics:metrics-core' implementation 'javax.ws.rs:javax.ws.rs-api' + implementation 'org.jetbrains:annotations' implementation 'org.apache.commons:commons-lang3' implementation 'org.eclipse.collections:eclipse-collections' implementation 'org.eclipse.collections:eclipse-collections-api' diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultDeleteExecutor.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultDeleteExecutor.java index efc7a41eacd..7809fb88c64 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultDeleteExecutor.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultDeleteExecutor.java @@ -22,6 +22,7 @@ import com.palantir.atlasdb.keyvalue.api.KeyValueService; import com.palantir.atlasdb.keyvalue.api.TableReference; import com.palantir.atlasdb.logging.LoggingArgs; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; import com.palantir.logsafe.SafeArg; import com.palantir.logsafe.UnsafeArg; import com.palantir.logsafe.logger.SafeLogger; @@ -50,6 +51,10 @@ public DefaultDeleteExecutor(KeyValueService keyValueService, ExecutorService ex this.executorService = executorService; } + public static DeleteExecutor createDefault(KeyValueService keyValueService) { + return new DefaultDeleteExecutor(keyValueService, DefaultTaskExecutors.createDefaultDeleteExecutor()); + } + @Override public void scheduleForDeletion(TableReference tableRef, Map keysToDelete) { if (keysToDelete.isEmpty()) { diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultKeyValueSnapshotReader.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultKeyValueSnapshotReader.java new file mode 100644 index 00000000000..6cc7ce5ce04 --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultKeyValueSnapshotReader.java @@ -0,0 +1,408 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl; + +import com.google.common.base.Predicates; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; +import com.google.common.collect.Streams; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; +import com.palantir.atlasdb.AtlasDbConstants; +import com.palantir.atlasdb.cell.api.TransactionKeyValueService; +import com.palantir.atlasdb.futures.AtlasFutures; +import com.palantir.atlasdb.keyvalue.api.Cell; +import com.palantir.atlasdb.keyvalue.api.ColumnSelection; +import com.palantir.atlasdb.keyvalue.api.KeyAlreadyExistsException; +import com.palantir.atlasdb.keyvalue.api.RowResult; +import com.palantir.atlasdb.keyvalue.api.TableReference; +import com.palantir.atlasdb.keyvalue.api.Value; +import com.palantir.atlasdb.keyvalue.impl.Cells; +import com.palantir.atlasdb.keyvalue.impl.RowResults; +import com.palantir.atlasdb.logging.LoggingArgs; +import com.palantir.atlasdb.tracing.TraceStatistics; +import com.palantir.atlasdb.transaction.api.CommitTimestampLoader; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotEventRecorder; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReader; +import com.palantir.atlasdb.transaction.api.TransactionFailedRetriableException; +import com.palantir.atlasdb.transaction.service.TransactionService; +import com.palantir.common.annotation.Output; +import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.UnsafeArg; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; +import com.palantir.logsafe.exceptions.SafeRuntimeException; +import com.palantir.logsafe.logger.SafeLogger; +import com.palantir.logsafe.logger.SafeLoggerFactory; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.NavigableMap; +import java.util.Set; +import java.util.function.Function; +import java.util.function.LongSupplier; +import org.eclipse.collections.api.LongIterable; +import org.eclipse.collections.api.factory.primitive.LongSets; +import org.eclipse.collections.api.map.primitive.LongLongMap; +import org.eclipse.collections.api.set.primitive.ImmutableLongSet; +import org.eclipse.collections.api.set.primitive.LongSet; +import org.jetbrains.annotations.NotNull; + +public final class DefaultKeyValueSnapshotReader implements KeyValueSnapshotReader { + private static final SafeLogger log = SafeLoggerFactory.get(DefaultKeyValueSnapshotReader.class); + + private final TransactionKeyValueService transactionKeyValueService; + private final TransactionService transactionService; + private final CommitTimestampLoader commitTimestampLoader; + private final boolean allowHiddenTableAccess; + private final ReadSentinelHandler readSentinelHandler; + private final LongSupplier startTimestampSupplier; + private final IntraReadSnapshotValidator intraReadSnapshotValidator; + private final DeleteExecutor deleteExecutor; + private final KeyValueSnapshotEventRecorder eventRecorder; + + public DefaultKeyValueSnapshotReader( + TransactionKeyValueService transactionKeyValueService, + TransactionService transactionService, + CommitTimestampLoader commitTimestampLoader, + boolean allowHiddenTableAccess, + ReadSentinelHandler readSentinelHandler, + LongSupplier startTimestampSupplier, + IntraReadSnapshotValidator intraReadSnapshotValidator, + DeleteExecutor deleteExecutor, + KeyValueSnapshotEventRecorder eventRecorder) { + this.transactionKeyValueService = transactionKeyValueService; + this.transactionService = transactionService; + this.commitTimestampLoader = commitTimestampLoader; + this.allowHiddenTableAccess = allowHiddenTableAccess; + this.readSentinelHandler = readSentinelHandler; + this.startTimestampSupplier = startTimestampSupplier; + this.intraReadSnapshotValidator = intraReadSnapshotValidator; + this.deleteExecutor = deleteExecutor; + this.eventRecorder = eventRecorder; + } + + @Override + public ListenableFuture> getAsync(TableReference tableReference, Set cells) { + return getInternal(tableReference, cells); + } + + @Override + public NavigableMap> getRows( + TableReference tableReference, + Iterable rows, + ColumnSelection columnSelection, + ImmutableMap.Builder resultCollector) { + Map rawResults = new HashMap<>(transactionKeyValueService.getRows( + tableReference, rows, columnSelection, startTimestampSupplier.getAsLong())); + // We don't need to do work postFiltering if we have a write locally. + rawResults.keySet().removeAll(resultCollector.buildOrThrow().keySet()); + return filterRowResults(tableReference, rawResults, resultCollector); + } + + // public Map>> getRowsColumnRangeIterator( + // TableReference tableRef, Iterable rows, BatchColumnRangeSelection columnRangeSelection) { + // ImmutableSortedMap rawResults = ImmutableSortedMap.copyOf( + // transactionKeyValueService.getRowsColumnRange( + // tableRef, rows, columnRangeSelection, startTimestampSupplier.getAsLong()), + // PtBytes.BYTES_COMPARATOR); + // ImmutableSortedMap>> postFilteredResults = Streams.stream(rows) + // .collect(ImmutableSortedMap.toImmutableSortedMap(PtBytes.BYTES_COMPARATOR, row -> row, row -> { + // // explicitly not using Optional due to allocation perf overhead + // Iterator> entryIterator; + // RowColumnRangeIterator rawIterator = rawResults.get(row); + // if (rawIterator != null) { + // entryIterator = closer.register( + // getPostFilteredColumns(tableRef, columnRangeSelection, row, rawIterator)); + // } else { + // entryIterator = ClosableIterators.wrapWithEmptyClose( + // getLocalWritesForColumnRange(tableRef, columnRangeSelection, row) + // .entrySet() + // .iterator()); + // } + // return scopeToTransaction(entryIterator); + // })); + // return postFilteredResults; + // } + + @NotNull + private ListenableFuture> getInternal(TableReference tableReference, Set cells) { + Map timestampsByCell = Cells.constantValueMap(cells, startTimestampSupplier.getAsLong()); + ListenableFuture>> postFilteredResults = Futures.transformAsync( + transactionKeyValueService.getAsync(tableReference, timestampsByCell), + rawResults -> getWithPostFilteringAsync(tableReference, rawResults, Value.GET_VALUE), + MoreExecutors.directExecutor()); + + return Futures.transform(postFilteredResults, ImmutableMap::copyOf, MoreExecutors.directExecutor()); + } + + private ListenableFuture>> getWithPostFilteringAsync( + TableReference tableRef, Map rawResults, Function transformer) { + long bytes = 0; + for (Map.Entry entry : rawResults.entrySet()) { + bytes += entry.getValue().getContents().length + Cells.getApproxSizeOfCell(entry.getKey()); + } + if (bytes > TransactionConstants.WARN_LEVEL_FOR_QUEUED_BYTES && log.isWarnEnabled()) { + log.warn( + "A single get had quite a few bytes: {} for table {}. The number of results was {}. " + + "Enable debug logging for more information.", + SafeArg.of("numBytes", bytes), + LoggingArgs.tableRef(tableRef), + SafeArg.of("numResults", rawResults.size())); + if (log.isDebugEnabled()) { + log.debug( + "The first 10 results of your request were {}.", + UnsafeArg.of("results", Iterables.limit(rawResults.entrySet(), 10)), + new SafeRuntimeException("This exception and stack trace are provided for debugging purposes")); + } + eventRecorder.recordManyBytesReadForTable(tableRef, bytes); + } + + eventRecorder.recordCellsRead(tableRef, rawResults.size()); + + Collection> resultsAccumulator = new ArrayList<>(); + + if (AtlasDbConstants.HIDDEN_TABLES.contains(tableRef)) { + Preconditions.checkState(allowHiddenTableAccess, "hidden tables cannot be read in this transaction"); + // hidden tables are used outside of the transaction protocol, and in general have invalid timestamps, + // so do not apply post-filtering as post-filtering would rollback (actually delete) the data incorrectly + // this case is hit when reading a hidden table from console + for (Map.Entry e : rawResults.entrySet()) { + resultsAccumulator.add(Maps.immutableEntry(e.getKey(), transformer.apply(e.getValue()))); + } + return Futures.immediateFuture(resultsAccumulator); + } + + return Futures.transformAsync( + Futures.immediateFuture(rawResults), + resultsToPostFilter -> + getWithPostFilteringIterate(tableRef, resultsToPostFilter, resultsAccumulator, transformer), + MoreExecutors.directExecutor()); + } + + private ListenableFuture>> getWithPostFilteringIterate( + TableReference tableReference, + Map resultsToPostFilter, + Collection> resultsAccumulator, + Function transformer) { + return Futures.transformAsync( + Futures.immediateFuture(resultsToPostFilter), + results -> { + int iterations = 0; + Map remainingResultsToPostFilter = results; + while (!remainingResultsToPostFilter.isEmpty()) { + remainingResultsToPostFilter = AtlasFutures.getUnchecked(getWithPostFilteringInternal( + tableReference, remainingResultsToPostFilter, resultsAccumulator, transformer)); + Preconditions.checkState( + ++iterations < SnapshotTransaction.MAX_POST_FILTERING_ITERATIONS, + "Unable to filter cells to find correct result after " + + "reaching max iterations. This is likely due to aborted cells lying around," + + " or in the very rare case, could be due to transactions which constantly " + + "conflict but never commit. These values will be cleaned up eventually, but" + + " if the issue persists, ensure that sweep is caught up.", + SafeArg.of("table", tableReference), + SafeArg.of("maxIterations", SnapshotTransaction.MAX_POST_FILTERING_ITERATIONS)); + } + eventRecorder.recordCellsReturned(tableReference, resultsAccumulator.size()); + return Futures.immediateFuture(resultsAccumulator); + }, + MoreExecutors.directExecutor()); + } + + private ListenableFuture> getWithPostFilteringInternal( + TableReference tableRef, + Map rawResults, + @Output Collection> resultsCollector, + Function transformer) { + Set orphans = readSentinelHandler.findAndMarkOrphanedSweepSentinelsForDeletion( + transactionKeyValueService, tableRef, rawResults); + LongSet valuesStartTimestamps = getStartTimestampsForValues(rawResults.values()); + + return Futures.transformAsync( + getCommitTimestamps(tableRef, valuesStartTimestamps), + commitTimestamps -> collectCellsToPostFilter( + tableRef, rawResults, resultsCollector, transformer, commitTimestamps, orphans), + MoreExecutors.directExecutor()); + } + + private ListenableFuture> collectCellsToPostFilter( + TableReference tableRef, + Map rawResults, + @Output Collection> resultsCollector, + Function transformer, + LongLongMap commitTimestamps, + Set knownOrphans) { + Map keysToReload = Maps.newHashMapWithExpectedSize(0); + Map keysToDelete = Maps.newHashMapWithExpectedSize(0); + ImmutableSet.Builder keysAddedBuilder = ImmutableSet.builder(); + + for (Map.Entry e : rawResults.entrySet()) { + Cell key = e.getKey(); + Value value = e.getValue(); + + if (isSweepSentinel(value) && !knownOrphans.contains(key)) { + eventRecorder.recordFilteredSweepSentinel(tableRef); + + // This means that this transaction started too long ago. When we do garbage collection, + // we clean up old values, and this transaction started at a timestamp before the garbage collection. + readSentinelHandler.handleReadSentinel(); + } else { + long theirCommitTimestamp = + commitTimestamps.getIfAbsent(value.getTimestamp(), TransactionConstants.FAILED_COMMIT_TS); + if (theirCommitTimestamp == TransactionConstants.FAILED_COMMIT_TS) { + keysToReload.put(key, value.getTimestamp()); + + // This is from a failed transaction so we can roll it back and then reload it. + keysToDelete.put(key, value.getTimestamp()); + eventRecorder.recordFilteredUncommittedTransaction(tableRef); + } else if (theirCommitTimestamp > startTimestampSupplier.getAsLong()) { + // The value's commit timestamp is after our start timestamp. + // This means the value is from a transaction which committed + // after our transaction began. We need to try reading at an + // earlier timestamp. + keysToReload.put(key, value.getTimestamp()); + eventRecorder.recordFilteredTransactionCommittingAfterOurStart(tableRef); + } else { + // The value has a commit timestamp less than our start timestamp, and is visible and valid. + if (value.getContents().length != 0) { + resultsCollector.add(Maps.immutableEntry(key, transformer.apply(value))); + keysAddedBuilder.add(key); + } + } + } + } + Set keysAddedToResults = keysAddedBuilder.build(); + + if (!keysToDelete.isEmpty()) { + // if we can't roll back the failed transactions, we should just try again + if (!rollbackFailedTransactions(tableRef, keysToDelete, commitTimestamps)) { + return Futures.immediateFuture(getRemainingResults(rawResults, keysAddedToResults)); + } + } + + if (!keysToReload.isEmpty()) { + // TODO (jkong): Consider removing when a decision on validating pre-commit requirements mid-read is made + return Futures.transform( + transactionKeyValueService.getAsync(tableRef, keysToReload), + nextRawResults -> { + boolean allPossibleCellsReadAndPresent = nextRawResults.size() == keysToReload.size(); + intraReadSnapshotValidator.validateInternalSnapshot( + tableRef, startTimestampSupplier, allPossibleCellsReadAndPresent); + return getRemainingResults(nextRawResults, keysAddedToResults); + }, + MoreExecutors.directExecutor()); + } + return Futures.immediateFuture(ImmutableMap.of()); + } + + private Map getRemainingResults(Map rawResults, Set keysAddedToResults) { + Map remainingResults = new HashMap<>(rawResults); + remainingResults.keySet().removeAll(keysAddedToResults); + return remainingResults; + } + + private LongSet getStartTimestampsForValues(Iterable values) { + return LongSets.immutable.withAll(Streams.stream(values).mapToLong(Value::getTimestamp)); + } + + private ListenableFuture getCommitTimestamps(TableReference tableRef, LongIterable startTimestamps) { + return commitTimestampLoader.getCommitTimestamps(tableRef, startTimestamps, true); + } + + /** + * This will attempt to rollback the passed transactions. If all are rolled back correctly this method will also + * delete the values for the transactions that have been rolled back. + * + * @return false if we cannot roll back the failed transactions because someone beat us to it + */ + private boolean rollbackFailedTransactions( + TableReference tableRef, Map keysToDelete, LongLongMap commitTimestamps) { + ImmutableLongSet timestamps = LongSets.immutable.ofAll(keysToDelete.values()); + boolean allRolledBack = timestamps.allSatisfy(startTs -> { + if (commitTimestamps.containsKey(startTs)) { + long commitTs = commitTimestamps.get(startTs); + if (commitTs != TransactionConstants.FAILED_COMMIT_TS) { + throw new SafeIllegalArgumentException( + "Cannot rollback already committed transaction", + SafeArg.of("startTs", startTs), + SafeArg.of("commitTs", commitTs)); + } + return true; + } + log.warn("Rolling back transaction: {}", SafeArg.of("startTs", startTs)); + return rollbackOtherTransaction(startTs); + }); + + if (allRolledBack) { + deleteExecutor.scheduleForDeletion(tableRef, keysToDelete); + } + return allRolledBack; + } + + private boolean rollbackOtherTransaction(long startTs) { + try { + transactionService.putUnlessExists(startTs, TransactionConstants.FAILED_COMMIT_TS); + eventRecorder.recordRolledBackOtherTransaction(); + return true; + } catch (KeyAlreadyExistsException e) { + log.debug( + "This isn't a bug but it should be very infrequent. Two transactions tried to roll back someone" + + " else's request with start: {}", + SafeArg.of("startTs", startTs), + new TransactionFailedRetriableException( + "Two transactions tried to roll back someone else's request with start: " + startTs, e)); + return false; + } + } + + private NavigableMap> filterRowResults( + TableReference tableRef, Map rawResults, ImmutableMap.Builder resultCollector) { + ImmutableMap collected = resultCollector + .putAll(getWithPostFilteringSync(tableRef, rawResults, Value.GET_VALUE)) + .buildOrThrow(); + Map filterDeletedValues = removeEmptyColumns(collected, tableRef); + return RowResults.viewOfSortedMap(Cells.breakCellsUpByRow(filterDeletedValues)); + } + + private Collection> getWithPostFilteringSync( + TableReference tableRef, Map rawResults, Function transformer) { + return AtlasFutures.getUnchecked(getWithPostFilteringAsync(tableRef, rawResults, transformer)); + } + + private Map removeEmptyColumns(Map unfiltered, TableReference tableReference) { + Map filtered = Maps.filterValues(unfiltered, Predicates.not(Value::isTombstone)); + + int emptyValues = unfiltered.size() - filtered.size(); + eventRecorder.recordEmptyValueRead(tableReference); + TraceStatistics.incEmptyValues(emptyValues); + + return filtered; + } + + private static boolean isSweepSentinel(Value value) { + return value.getTimestamp() == Value.INVALID_VALUE_TIMESTAMP; + } + + interface CellGetter { + ListenableFuture> get(TableReference tableRef, Map timestampByCell); + } +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultKeyValueSnapshotReaderManager.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultKeyValueSnapshotReaderManager.java new file mode 100644 index 00000000000..504f4d28317 --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultKeyValueSnapshotReaderManager.java @@ -0,0 +1,67 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl; + +import com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReader; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager; +import com.palantir.atlasdb.transaction.api.OrphanedSentinelDeleter; +import com.palantir.atlasdb.transaction.service.TransactionService; + +public class DefaultKeyValueSnapshotReaderManager implements KeyValueSnapshotReaderManager { + private final TransactionKeyValueServiceManager transactionKeyValueServiceManager; + private final TransactionService transactionService; + private final boolean allowHiddenTableAccess; + private final OrphanedSentinelDeleter orphanedSentinelDeleter; + private final DeleteExecutor deleteExecutor; + + public DefaultKeyValueSnapshotReaderManager( + TransactionKeyValueServiceManager transactionKeyValueServiceManager, + TransactionService transactionService, + boolean allowHiddenTableAccess, + OrphanedSentinelDeleter orphanedSentinelDeleter, + DeleteExecutor deleteExecutor) { + this.transactionKeyValueServiceManager = transactionKeyValueServiceManager; + this.transactionService = transactionService; + this.allowHiddenTableAccess = allowHiddenTableAccess; + this.orphanedSentinelDeleter = orphanedSentinelDeleter; + this.deleteExecutor = deleteExecutor; + } + + @Override + public KeyValueSnapshotReader createKeyValueSnapshotReader(TransactionContext transactionContext) { + return new DefaultKeyValueSnapshotReader( + transactionKeyValueServiceManager.getTransactionKeyValueService( + transactionContext.startTimestampSupplier()), + transactionService, + transactionContext.commitTimestampLoader(), + allowHiddenTableAccess, + // TODO (jkong): The allocations here feel wasteful. Should we have a cache of some kind? + new ReadSentinelHandler( + transactionService, + transactionContext.transactionReadSentinelBehavior(), + orphanedSentinelDeleter), + transactionContext.startTimestampSupplier(), + (tableReference, timestampSupplier, allCellsReadAndPresent) -> transactionContext + .preCommitRequirementValidator() + .throwIfPreCommitRequirementsNotMetOnRead( + tableReference, timestampSupplier.getAsLong(), allCellsReadAndPresent), + deleteExecutor, + transactionContext.keyValueSnapshotEventRecorder()); + } +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultLockRefresher.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultLockRefresher.java new file mode 100644 index 00000000000..df1070daac3 --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultLockRefresher.java @@ -0,0 +1,34 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl; + +import com.palantir.lock.v2.LockToken; +import com.palantir.lock.v2.TimelockService; +import java.util.Set; + +public class DefaultLockRefresher implements LockRefresher { + private final TimelockService timelockService; + + public DefaultLockRefresher(TimelockService timelockService) { + this.timelockService = timelockService; + } + + @Override + public Set refreshLocks(Set tokensToRefresh) { + return timelockService.refreshLockLeases(tokensToRefresh); + } +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultOrphanedSentinelDeleter.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultOrphanedSentinelDeleter.java new file mode 100644 index 00000000000..19ae28c9bb6 --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultOrphanedSentinelDeleter.java @@ -0,0 +1,54 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl; + +import com.palantir.atlasdb.keyvalue.api.Cell; +import com.palantir.atlasdb.keyvalue.api.TableReference; +import com.palantir.atlasdb.keyvalue.api.Value; +import com.palantir.atlasdb.table.description.SweepStrategy; +import com.palantir.atlasdb.table.description.SweeperStrategy; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; +import com.palantir.atlasdb.transaction.api.OrphanedSentinelDeleter; +import com.palantir.common.streams.KeyedStream; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; + +public final class DefaultOrphanedSentinelDeleter implements OrphanedSentinelDeleter { + private final Function sweepStrategyProvider; + private final DeleteExecutor deleteExecutor; + + public DefaultOrphanedSentinelDeleter( + Function sweepStrategyProvider, DeleteExecutor deleteExecutor) { + this.sweepStrategyProvider = sweepStrategyProvider; + this.deleteExecutor = deleteExecutor; + } + + @Override + public void scheduleSentinelsForDeletion(TableReference tableReference, Set orphanedSentinels) { + if (sweepStrategyProvider + .apply(tableReference) + .getSweeperStrategy() + .map(sweeperStrategy -> sweeperStrategy == SweeperStrategy.THOROUGH) + .orElse(false)) { + Map sentinels = KeyedStream.of(orphanedSentinels) + .map(_ignore -> Value.INVALID_VALUE_TIMESTAMP) + .collectToMap(); + deleteExecutor.scheduleForDeletion(tableReference, sentinels); + } + } +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultPreCommitRequirementValidator.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultPreCommitRequirementValidator.java new file mode 100644 index 00000000000..7cad9570033 --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/DefaultPreCommitRequirementValidator.java @@ -0,0 +1,149 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl; + +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; +import com.palantir.atlasdb.keyvalue.api.Cell; +import com.palantir.atlasdb.keyvalue.api.TableReference; +import com.palantir.atlasdb.transaction.TransactionConfig; +import com.palantir.atlasdb.transaction.api.PreCommitCondition; +import com.palantir.atlasdb.transaction.api.PreCommitRequirementValidator; +import com.palantir.atlasdb.transaction.api.TransactionFailedException; +import com.palantir.atlasdb.transaction.api.TransactionLockTimeoutException; +import com.palantir.atlasdb.transaction.impl.metrics.TransactionOutcomeMetrics; +import com.palantir.lock.v2.LockToken; +import com.palantir.logsafe.UnsafeArg; +import com.palantir.logsafe.logger.SafeLogger; +import com.palantir.logsafe.logger.SafeLoggerFactory; +import java.util.HashSet; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.Supplier; + +// Transaction scoped. One instance should live for the duration of a transaction. +public class DefaultPreCommitRequirementValidator implements PreCommitRequirementValidator { + private static final SafeLogger log = SafeLoggerFactory.get(DefaultPreCommitRequirementValidator.class); + + private final PreCommitCondition userPreCommitCondition; + private final SweepStrategyManager sweepStrategyManager; + private final Supplier transactionConfig; + private final LockRefresher lockRefresher; + private final Optional immutableTimestampLock; + private final boolean validateLocksOnReads; + private final TransactionOutcomeMetrics transactionOutcomeMetrics; + + public DefaultPreCommitRequirementValidator( + PreCommitCondition userPreCommitCondition, + SweepStrategyManager sweepStrategyManager, + Supplier transactionConfig, + LockRefresher lockRefresher, + Optional immutableTimestampLock, + boolean validateLocksOnReads, + TransactionOutcomeMetrics transactionOutcomeMetrics) { + this.userPreCommitCondition = userPreCommitCondition; + this.sweepStrategyManager = sweepStrategyManager; + this.transactionConfig = transactionConfig; + this.lockRefresher = lockRefresher; + this.immutableTimestampLock = immutableTimestampLock; + this.validateLocksOnReads = validateLocksOnReads; + this.transactionOutcomeMetrics = transactionOutcomeMetrics; + } + + // TODO (jkong): The boolean means "are there unvalidated reads"? + @Override + public boolean throwIfPreCommitRequirementsNotMetOnRead( + TableReference tableRef, long timestamp, boolean allPossibleCellsReadAndPresent) { + if (isValidationNecessaryOnReads(tableRef, allPossibleCellsReadAndPresent)) { + throwIfPreCommitRequirementsNotMet(Optional.empty(), timestamp); + } + return !allPossibleCellsReadAndPresent; + } + + @Override + public void throwIfPreCommitConditionInvalidAtCommitOnWriteTransaction( + Map> mutations, long timestamp) { + try { + userPreCommitCondition.throwIfConditionInvalid(mutations, timestamp); + } catch (TransactionFailedException ex) { + transactionOutcomeMetrics.markPreCommitCheckFailed(); + throw ex; + } + } + + @Override + public void throwIfImmutableTsOrCommitLocksExpired(Optional commitLocksToken) { + Set expiredLocks = refreshCommitAndImmutableTsLocks(commitLocksToken); + if (!expiredLocks.isEmpty()) { + final String baseMsg = "Locks acquired as part of the transaction protocol are no longer valid. "; + String expiredLocksErrorString = getExpiredLocksErrorString(commitLocksToken, expiredLocks); + TransactionLockTimeoutException ex = new TransactionLockTimeoutException(baseMsg + expiredLocksErrorString); + log.warn(baseMsg + "{}", UnsafeArg.of("expiredLocksErrorString", expiredLocksErrorString), ex); + transactionOutcomeMetrics.markLocksExpired(); + throw ex; + } + } + + @Override + public void throwIfPreCommitRequirementsNotMet(Optional commitLocksToken, long timestamp) { + throwIfImmutableTsOrCommitLocksExpired(commitLocksToken); + throwIfPreCommitConditionInvalid(timestamp); + } + + @Override + public void throwIfPreCommitConditionInvalid(long timestamp) { + try { + userPreCommitCondition.throwIfConditionInvalid(timestamp); + } catch (TransactionFailedException ex) { + transactionOutcomeMetrics.markPreCommitCheckFailed(); + throw ex; + } + } + + private boolean isValidationNecessaryOnReads(TableReference tableRef, boolean allPossibleCellsReadAndPresent) { + return validateLocksOnReads && requiresImmutableTimestampLocking(tableRef, allPossibleCellsReadAndPresent); + } + + private boolean requiresImmutableTimestampLocking(TableReference tableRef, boolean allPossibleCellsReadAndPresent) { + return sweepStrategyManager.get(tableRef).mustCheckImmutableLock(allPossibleCellsReadAndPresent) + || transactionConfig.get().lockImmutableTsOnReadOnlyTransactions(); + } + + private String getExpiredLocksErrorString(Optional commitLocksToken, Set expiredLocks) { + return "The following immutable timestamp lock was required: " + immutableTimestampLock + + "; the following commit locks were required: " + commitLocksToken + + "; the following locks are no longer valid: " + expiredLocks; + } + + /** + * Refreshes external and commit locks. + * + * @return set of locks that could not be refreshed + */ + private Set refreshCommitAndImmutableTsLocks(Optional commitLocksToken) { + Set toRefresh = new HashSet<>(); + commitLocksToken.ifPresent(toRefresh::add); + immutableTimestampLock.ifPresent(toRefresh::add); + + if (toRefresh.isEmpty()) { + return ImmutableSet.of(); + } + + return Sets.difference(toRefresh, lockRefresher.refreshLocks(toRefresh)).immutableCopy(); + } +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/IntraReadSnapshotValidator.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/IntraReadSnapshotValidator.java new file mode 100644 index 00000000000..623e96773d9 --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/IntraReadSnapshotValidator.java @@ -0,0 +1,29 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl; + +import com.palantir.atlasdb.keyvalue.api.TableReference; +import java.util.function.LongSupplier; + +public interface IntraReadSnapshotValidator { + + /** + * Checks that a snapshot read is still valid, throwing an exception if it is not. + */ + void validateInternalSnapshot( + TableReference tableReference, LongSupplier startTimestampSupplier, boolean allPossibleCellsReadAndPresent); +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/LockRefresher.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/LockRefresher.java new file mode 100644 index 00000000000..7d1f3f117a5 --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/LockRefresher.java @@ -0,0 +1,30 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl; + +import com.palantir.lock.v2.LockToken; +import java.util.Set; + +/** + * Facade of {@link com.palantir.lock.v2.TimelockService} that only allows for refreshing of locks. + */ +public interface LockRefresher { + /** + * Returns lock tokens from the provided set that were successfully refreshed. + */ + Set refreshLocks(Set tokensToRefresh); +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/ReadSentinelHandler.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/ReadSentinelHandler.java new file mode 100644 index 00000000000..de455a6653f --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/ReadSentinelHandler.java @@ -0,0 +1,131 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl; + +import com.google.common.collect.Maps; +import com.palantir.atlasdb.cell.api.TransactionKeyValueService; +import com.palantir.atlasdb.keyvalue.api.Cell; +import com.palantir.atlasdb.keyvalue.api.TableReference; +import com.palantir.atlasdb.keyvalue.api.Value; +import com.palantir.atlasdb.transaction.api.OrphanedSentinelDeleter; +import com.palantir.atlasdb.transaction.api.Transaction.TransactionType; +import com.palantir.atlasdb.transaction.api.TransactionFailedRetriableException; +import com.palantir.atlasdb.transaction.api.TransactionReadSentinelBehavior; +import com.palantir.atlasdb.transaction.service.TransactionService; +import com.palantir.common.streams.KeyedStream; +import com.palantir.logsafe.SafeArg; +import com.palantir.logsafe.exceptions.SafeIllegalStateException; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + +public final class ReadSentinelHandler { + private final TransactionService transactionService; + private final TransactionReadSentinelBehavior readSentinelBehavior; + private final OrphanedSentinelDeleter orphanedSentinelDeleter; + + public ReadSentinelHandler( + TransactionService transactionService, + TransactionReadSentinelBehavior readSentinelBehavior, + OrphanedSentinelDeleter orphanedSentinelDeleter) { + this.transactionService = transactionService; + this.readSentinelBehavior = readSentinelBehavior; + this.orphanedSentinelDeleter = orphanedSentinelDeleter; + } + + /** + * A sentinel becomes orphaned if the table has been truncated between the time where the write occurred and where + * it was truncated. In this case, there is a chance that we end up with a sentinel with no valid AtlasDB cell + * covering it. In this case, we ignore it. + */ + public Set findAndMarkOrphanedSweepSentinelsForDeletion( + TransactionKeyValueService transactionKeyValueService, TableReference table, Map rawResults) { + Set sweepSentinels = Maps.filterValues(rawResults, ReadSentinelHandler::isSweepSentinel) + .keySet(); + if (sweepSentinels.isEmpty()) { + return sweepSentinels; + } + + // for each sentinel, start at long max. Then iterate down with each found uncommitted value. + // if committed value seen, stop: the sentinel is not orphaned + // if we get back -1, the sentinel is orphaned + Map timestampCandidates = new HashMap<>( + transactionKeyValueService.getLatestTimestamps(table, Maps.asMap(sweepSentinels, x -> Long.MAX_VALUE))); + Set actualOrphanedSentinels = new HashSet<>(); + + while (!timestampCandidates.isEmpty()) { + Map> sentinelTypeToTimestamps = timestampCandidates.entrySet().stream() + .collect(Collectors.groupingBy( + entry -> entry.getValue() == Value.INVALID_VALUE_TIMESTAMP + ? SentinelType.DEFINITE_ORPHANED + : SentinelType.INDETERMINATE, + Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))); + + Map definiteOrphans = sentinelTypeToTimestamps.get(SentinelType.DEFINITE_ORPHANED); + if (definiteOrphans != null) { + actualOrphanedSentinels.addAll(definiteOrphans.keySet()); + } + + Map cellsToQuery = sentinelTypeToTimestamps.get(SentinelType.INDETERMINATE); + if (cellsToQuery == null) { + break; + } + Set committedStartTimestamps = KeyedStream.stream(transactionService.get(cellsToQuery.values())) + .filter(Objects::nonNull) + .keys() + .collect(Collectors.toSet()); + + Map nextTimestampCandidates = KeyedStream.stream(cellsToQuery) + .filter(cellStartTimestamp -> !committedStartTimestamps.contains(cellStartTimestamp)) + .collectToMap(); + timestampCandidates = transactionKeyValueService.getLatestTimestamps(table, nextTimestampCandidates); + } + + orphanedSentinelDeleter.scheduleSentinelsForDeletion(table, actualOrphanedSentinels); + return actualOrphanedSentinels; + } + + public void handleReadSentinel() { + // This means that this transaction started too long ago. When we do garbage collection, + // we clean up old values, and this transaction started at a timestamp before the garbage collection. + switch (readSentinelBehavior) { + case IGNORE: + break; + case THROW_EXCEPTION: + throw new TransactionFailedRetriableException("Tried to read a value that has been " + + "deleted. This can be caused by hard delete transactions using the type " + + TransactionType.AGGRESSIVE_HARD_DELETE + + ". It can also be caused by transactions taking too long, or" + + " its locks expired. Retrying it should work."); + default: + throw new SafeIllegalStateException( + "Invalid read sentinel behavior", SafeArg.of("readSentinelBehaviour", readSentinelBehavior)); + } + } + + private static boolean isSweepSentinel(Value value) { + return value.getTimestamp() == Value.INVALID_VALUE_TIMESTAMP; + } + + private enum SentinelType { + DEFINITE_ORPHANED, + INDETERMINATE; + } +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/ReadValidationCommitTimestampLoader.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/ReadValidationCommitTimestampLoader.java new file mode 100644 index 00000000000..699108523c5 --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/ReadValidationCommitTimestampLoader.java @@ -0,0 +1,143 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl; + +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; +import com.palantir.atlasdb.futures.AtlasFutures; +import com.palantir.atlasdb.keyvalue.api.TableReference; +import com.palantir.atlasdb.transaction.api.CommitTimestampLoader; +import com.palantir.atlasdb.transaction.api.TransactionSerializableConflictException; +import com.palantir.atlasdb.transaction.impl.SerializableTransaction.PartitionedTimestamps; +import com.palantir.atlasdb.transaction.impl.metrics.TransactionOutcomeMetrics; +import com.palantir.atlasdb.transaction.service.AsyncTransactionService; +import com.palantir.atlasdb.transaction.service.TransactionService; +import org.eclipse.collections.api.LongIterable; +import org.eclipse.collections.api.factory.primitive.LongLongMaps; +import org.eclipse.collections.api.factory.primitive.LongSets; +import org.eclipse.collections.api.map.primitive.LongLongMap; +import org.eclipse.collections.api.map.primitive.MutableLongLongMap; +import org.eclipse.collections.api.set.primitive.LongSet; +import org.eclipse.collections.api.set.primitive.MutableLongSet; +import org.jetbrains.annotations.Nullable; + +/** + * Loads commit timestamps for read validation, considering a simulated state of the world where the serializable + * transaction that we're performing validation for has already committed. + */ +public final class ReadValidationCommitTimestampLoader implements CommitTimestampLoader { + private final CommitTimestampLoader delegate; + private final TransactionService transactionService; + private final long startTs; + private final long commitTs; + private final TransactionOutcomeMetrics transactionOutcomeMetrics; + + public ReadValidationCommitTimestampLoader( + CommitTimestampLoader delegate, + TransactionService transactionService, + long startTs, + long commitTs, + TransactionOutcomeMetrics transactionOutcomeMetrics) { + this.delegate = delegate; + this.transactionService = transactionService; + this.startTs = startTs; + this.commitTs = commitTs; + this.transactionOutcomeMetrics = transactionOutcomeMetrics; + } + + @Override + public ListenableFuture getCommitTimestamps( + @Nullable TableReference tableRef, LongIterable startTimestamps, boolean shouldWaitForCommitterToComplete) { + PartitionedTimestamps partitionedTimestamps = splitTransactionBeforeAndAfter(startTs, startTimestamps); + + ListenableFuture postStartCommitTimestamps = getCommitTimestampsForTransactionsStartedAfterMe( + tableRef, transactionService, partitionedTimestamps.afterStart()); + + // We are ok to block here because if there is a cycle of transactions that could result in a deadlock, + // then at least one of them will be in the ab + ListenableFuture preStartCommitTimestamps = delegate.getCommitTimestamps( + tableRef, partitionedTimestamps.beforeStart(), shouldWaitForCommitterToComplete); + + return Futures.whenAllComplete(postStartCommitTimestamps, preStartCommitTimestamps) + .call( + () -> { + MutableLongLongMap map = + LongLongMaps.mutable.withAll(AtlasFutures.getDone(preStartCommitTimestamps)); + map.putAll(AtlasFutures.getDone(postStartCommitTimestamps)); + map.putAll(partitionedTimestamps.myCommittedTransaction()); + return map.toImmutable(); + }, + MoreExecutors.directExecutor()); + } + + private ListenableFuture getCommitTimestampsForTransactionsStartedAfterMe( + TableReference tableRef, AsyncTransactionService asyncTransactionService, LongSet startTimestamps) { + if (startTimestamps.isEmpty()) { + return Futures.immediateFuture(LongLongMaps.immutable.empty()); + } + + return Futures.transform( + // We do not block when waiting for results that were written after our start timestamp. + // If we block here it may lead to deadlock if two transactions (or a cycle of any length) have + // all written their data and all doing checks before committing. + delegate.getCommitTimestamps(tableRef, startTimestamps, false), + startToCommitTimestamps -> { + if (startToCommitTimestamps.keySet().containsAll(startTimestamps)) { + return startToCommitTimestamps; + } + // If we do not get back all these results we may be in the deadlock case so we + // should just fail out early. It may be the case that abort more transactions + // than needed to break the deadlock cycle, but this should be pretty rare. + transactionOutcomeMetrics.markReadWriteConflict(tableRef); + throw new TransactionSerializableConflictException( + "An uncommitted conflicting read was " + + "written after our start timestamp for table " + + tableRef + ". " + + "This case can cause deadlock and is very likely to be a " + + "read write conflict.", + tableRef); + }, + MoreExecutors.directExecutor()); + } + + /** + * Partitions {@code startTimestamps} in two sets, based on their relation to the start timestamp provided. + * + * @param myStart start timestamp of this transaction + * @param startTimestamps of transactions we are interested in + * @return a {@link PartitionedTimestamps} object containing split timestamps + */ + private PartitionedTimestamps splitTransactionBeforeAndAfter(long myStart, LongIterable startTimestamps) { + ImmutablePartitionedTimestamps.Builder builder = + ImmutablePartitionedTimestamps.builder().myCommitTimestamp(commitTs); + MutableLongSet beforeStart = LongSets.mutable.empty(); + MutableLongSet afterStart = LongSets.mutable.empty(); + startTimestamps.forEach(startTimestamp -> { + if (startTimestamp == myStart) { + builder.splittingStartTimestamp(myStart); + } else if (startTimestamp < myStart) { + beforeStart.add(startTimestamp); + } else { + afterStart.add(startTimestamp); + } + }); + builder.beforeStart(beforeStart.asUnmodifiable()); + builder.afterStart(afterStart.asUnmodifiable()); + return builder.build(); + } +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SerializableTransaction.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SerializableTransaction.java index 5a73ea6addc..2b380989fe5 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SerializableTransaction.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SerializableTransaction.java @@ -42,7 +42,6 @@ import com.palantir.atlasdb.cleaner.api.Cleaner; import com.palantir.atlasdb.debug.ConflictTracer; import com.palantir.atlasdb.encoding.PtBytes; -import com.palantir.atlasdb.futures.AtlasFutures; import com.palantir.atlasdb.keyvalue.api.BatchColumnRangeSelection; import com.palantir.atlasdb.keyvalue.api.Cell; import com.palantir.atlasdb.keyvalue.api.ColumnRangeSelection; @@ -60,7 +59,10 @@ import com.palantir.atlasdb.transaction.api.AtlasDbConstraintCheckingMode; import com.palantir.atlasdb.transaction.api.CommitTimestampLoader; import com.palantir.atlasdb.transaction.api.ConflictHandler; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager; import com.palantir.atlasdb.transaction.api.PreCommitCondition; +import com.palantir.atlasdb.transaction.api.PreCommitRequirementValidator; import com.palantir.atlasdb.transaction.api.Transaction; import com.palantir.atlasdb.transaction.api.TransactionReadSentinelBehavior; import com.palantir.atlasdb.transaction.api.TransactionSerializableConflictException; @@ -113,13 +115,9 @@ import java.util.function.Supplier; import java.util.stream.Collectors; import javax.annotation.Nullable; -import org.eclipse.collections.api.LongIterable; import org.eclipse.collections.api.factory.primitive.LongLongMaps; -import org.eclipse.collections.api.factory.primitive.LongSets; import org.eclipse.collections.api.map.primitive.LongLongMap; -import org.eclipse.collections.api.map.primitive.MutableLongLongMap; import org.eclipse.collections.api.set.primitive.LongSet; -import org.eclipse.collections.api.set.primitive.MutableLongSet; import org.immutables.value.Value; /** @@ -174,7 +172,9 @@ public SerializableTransaction( ConflictTracer conflictTracer, TableLevelMetricsController tableLevelMetricsController, TransactionKnowledgeComponents knowledge, - CommitTimestampLoader commitTimestampLoader) { + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager, + CommitTimestampLoader commitTimestampLoader, + PreCommitRequirementValidator preCommitRequirementValidator) { super( metricsManager, keyValueService, @@ -187,7 +187,6 @@ public SerializableTransaction( sweepStrategyManager, immutableTimestamp, immutableTsLock, - preCommitCondition, constraintCheckingMode, transactionTimeoutMillis, readSentinelBehavior, @@ -202,7 +201,9 @@ public SerializableTransaction( conflictTracer, tableLevelMetricsController, knowledge, - commitTimestampLoader); + keyValueSnapshotReaderManager, + commitTimestampLoader, + preCommitRequirementValidator); } @Override @@ -914,6 +915,8 @@ private NavigableMap getReadsInRange(TableReference table, RangeRe } private Transaction getReadOnlyTransaction(final long commitTs) { + CommitTimestampLoader readValidationLoader = new ReadValidationCommitTimestampLoader( + commitTimestampLoader, defaultTransactionService, getTimestamp(), commitTs, transactionOutcomeMetrics); return new SnapshotTransaction( metricsManager, transactionKeyValueService, @@ -926,7 +929,6 @@ private Transaction getReadOnlyTransaction(final long commitTs) { sweepStrategyManager, immutableTimestamp, Optional.empty(), - PreCommitConditions.NO_OP, AtlasDbConstraintCheckingMode.NO_CONSTRAINT_CHECKING, transactionReadTimeoutMillis, getReadSentinelBehavior(), @@ -941,94 +943,13 @@ private Transaction getReadOnlyTransaction(final long commitTs) { conflictTracer, tableLevelMetricsController, knowledge, - // TODO (jkong): Remove when extracting a custom read-only commit timestamp loader - commitTimestampLoader) { + keyValueSnapshotReaderManager, + readValidationLoader, + preCommitRequirementValidator) { @Override protected TransactionScopedCache getCache() { return lockWatchManager.getReadOnlyTransactionScopedCache(SerializableTransaction.this.getTimestamp()); } - - @Override - protected ListenableFuture getCommitTimestamps( - TableReference tableRef, LongIterable startTimestamps, boolean shouldWaitForCommitterToComplete) { - long myStart = SerializableTransaction.this.getTimestamp(); - PartitionedTimestamps partitionedTimestamps = splitTransactionBeforeAndAfter(myStart, startTimestamps); - - ListenableFuture postStartCommitTimestamps = - getCommitTimestampsForTransactionsStartedAfterMe(tableRef, partitionedTimestamps.afterStart()); - - // We are ok to block here because if there is a cycle of transactions that could result in a deadlock, - // then at least one of them will be in the ab - ListenableFuture preStartCommitTimestamps = super.getCommitTimestamps( - tableRef, partitionedTimestamps.beforeStart(), shouldWaitForCommitterToComplete); - - return Futures.whenAllComplete(postStartCommitTimestamps, preStartCommitTimestamps) - .call( - () -> { - MutableLongLongMap map = LongLongMaps.mutable.withAll( - AtlasFutures.getDone(preStartCommitTimestamps)); - map.putAll(AtlasFutures.getDone(postStartCommitTimestamps)); - map.putAll(partitionedTimestamps.myCommittedTransaction()); - return map.toImmutable(); - }, - MoreExecutors.directExecutor()); - } - - private ListenableFuture getCommitTimestampsForTransactionsStartedAfterMe( - TableReference tableRef, LongSet startTimestamps) { - if (startTimestamps.isEmpty()) { - return Futures.immediateFuture(LongLongMaps.immutable.empty()); - } - - return Futures.transform( - // We do not block when waiting for results that were written after our start timestamp. - // If we block here it may lead to deadlock if two transactions (or a cycle of any length) have - // all written their data and all doing checks before committing. - super.getCommitTimestamps(tableRef, startTimestamps, false), - startToCommitTimestamps -> { - if (startToCommitTimestamps.keySet().containsAll(startTimestamps)) { - return startToCommitTimestamps; - } - // If we do not get back all these results we may be in the deadlock case so we - // should just fail out early. It may be the case that abort more transactions - // than needed to break the deadlock cycle, but this should be pretty rare. - transactionOutcomeMetrics.markReadWriteConflict(tableRef); - throw new TransactionSerializableConflictException( - "An uncommitted conflicting read was " - + "written after our start timestamp for table " - + tableRef + ". " - + "This case can cause deadlock and is very likely to be a " - + "read write conflict.", - tableRef); - }, - MoreExecutors.directExecutor()); - } - - /** - * Partitions {@code startTimestamps} in two sets, based on their relation to the start timestamp provided. - * - * @param myStart start timestamp of this transaction - * @param startTimestamps of transactions we are interested in - * @return a {@link PartitionedTimestamps} object containing split timestamps - */ - private PartitionedTimestamps splitTransactionBeforeAndAfter(long myStart, LongIterable startTimestamps) { - ImmutablePartitionedTimestamps.Builder builder = - ImmutablePartitionedTimestamps.builder().myCommitTimestamp(commitTs); - MutableLongSet beforeStart = LongSets.mutable.empty(); - MutableLongSet afterStart = LongSets.mutable.empty(); - startTimestamps.forEach(startTimestamp -> { - if (startTimestamp == myStart) { - builder.splittingStartTimestamp(myStart); - } else if (startTimestamp < myStart) { - beforeStart.add(startTimestamp); - } else { - afterStart.add(startTimestamp); - } - }); - builder.beforeStart(beforeStart.asUnmodifiable()); - builder.afterStart(afterStart.asUnmodifiable()); - return builder.build(); - } }; } diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SerializableTransactionManager.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SerializableTransactionManager.java index 8b6a07eb8eb..8c4402fbcd9 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SerializableTransactionManager.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SerializableTransactionManager.java @@ -29,6 +29,8 @@ import com.palantir.atlasdb.transaction.TransactionConfig; import com.palantir.atlasdb.transaction.api.AtlasDbConstraintCheckingMode; import com.palantir.atlasdb.transaction.api.AutoDelegate_TransactionManager; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager; import com.palantir.atlasdb.transaction.api.PreCommitCondition; import com.palantir.atlasdb.transaction.api.TransactionManager; import com.palantir.atlasdb.transaction.api.TransactionReadSentinelBehavior; @@ -246,7 +248,9 @@ public static TransactionManager createInstrumented( ConflictTracer conflictTracer, MetricsFilterEvaluationContext metricsFilterEvaluationContext, Optional sharedGetRangesPoolSize, - TransactionKnowledgeComponents knowledge) { + TransactionKnowledgeComponents knowledge, + DeleteExecutor deleteExecutor, + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager) { return create( metricsManager, transactionKeyValueServiceManager, @@ -275,7 +279,9 @@ public static TransactionManager createInstrumented( conflictTracer, metricsFilterEvaluationContext, sharedGetRangesPoolSize, - knowledge); + knowledge, + deleteExecutor, + keyValueSnapshotReaderManager); } public static TransactionManager create( @@ -303,7 +309,8 @@ public static TransactionManager create( ConflictTracer conflictTracer, MetricsFilterEvaluationContext metricsFilterEvaluationContext, Optional sharedGetRangesPoolSize, - TransactionKnowledgeComponents knowledge) { + TransactionKnowledgeComponents knowledge, + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager) { return create( metricsManager, transactionKeyValueServiceManager, @@ -331,7 +338,11 @@ public static TransactionManager create( conflictTracer, metricsFilterEvaluationContext, sharedGetRangesPoolSize, - knowledge); + knowledge, + new DefaultDeleteExecutor( + transactionKeyValueServiceManager.getKeyValueService().orElseThrow(), + DefaultTaskExecutors.createDefaultDeleteExecutor()), + keyValueSnapshotReaderManager); } public static TransactionManager create( @@ -360,7 +371,9 @@ public static TransactionManager create( ConflictTracer conflictTracer, MetricsFilterEvaluationContext metricsFilterEvaluationContext, Optional sharedGetRangesPoolSize, - TransactionKnowledgeComponents knowledge) { + TransactionKnowledgeComponents knowledge, + DeleteExecutor deleteExecutor, + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager) { return create( metricsManager, transactionKeyValueServiceManager, @@ -388,7 +401,9 @@ public static TransactionManager create( conflictTracer, metricsFilterEvaluationContext, sharedGetRangesPoolSize, - knowledge); + knowledge, + deleteExecutor, + keyValueSnapshotReaderManager); } private static TransactionManager create( @@ -418,7 +433,9 @@ private static TransactionManager create( ConflictTracer conflictTracer, MetricsFilterEvaluationContext metricsFilterEvaluationContext, Optional sharedGetRangesPoolSize, - TransactionKnowledgeComponents knowledge) { + TransactionKnowledgeComponents knowledge, + DeleteExecutor deleteExecutor, + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager) { TransactionManager transactionManager = new SerializableTransactionManager( metricsManager, transactionKeyValueServiceManager, @@ -436,16 +453,14 @@ private static TransactionManager create( concurrentGetRangesThreadPoolSize, defaultGetRangesConcurrency, sweepQueueWriter, - // TODO(jakubk): This will be updated in further PRs as it needs to use the same API as sweep. - new DefaultDeleteExecutor( - transactionKeyValueServiceManager.getKeyValueService().orElseThrow(), - DefaultTaskExecutors.createDefaultDeleteExecutor()), + deleteExecutor, validateLocksOnReads, transactionConfig, conflictTracer, metricsFilterEvaluationContext, sharedGetRangesPoolSize, - knowledge); + knowledge, + keyValueSnapshotReaderManager); if (shouldInstrument) { transactionManager = AtlasDbMetrics.instrumentTimed( @@ -482,6 +497,9 @@ public static SerializableTransactionManager createForTest( int defaultGetRangesConcurrency, MultiTableSweepQueueWriter sweepQueue, TransactionKnowledgeComponents knowledge) { + DeleteExecutor deleteExecutor = new DefaultDeleteExecutor( + transactionKeyValueServiceManager.getKeyValueService().orElseThrow(), + DefaultTaskExecutors.createDefaultDeleteExecutor()); return new SerializableTransactionManager( metricsManager, transactionKeyValueServiceManager, @@ -499,15 +517,19 @@ public static SerializableTransactionManager createForTest( concurrentGetRangesThreadPoolSize, defaultGetRangesConcurrency, sweepQueue, - new DefaultDeleteExecutor( - transactionKeyValueServiceManager.getKeyValueService().orElseThrow(), - DefaultTaskExecutors.createDefaultDeleteExecutor()), + deleteExecutor, true, () -> ImmutableTransactionConfig.builder().build(), ConflictTracer.NO_OP, DefaultMetricsFilterEvaluationContext.createDefault(), Optional.empty(), - knowledge); + knowledge, + new DefaultKeyValueSnapshotReaderManager( + transactionKeyValueServiceManager, + transactionService, + false, + new DefaultOrphanedSentinelDeleter(sweepStrategyManager::get, deleteExecutor), + deleteExecutor)); } public SerializableTransactionManager( @@ -533,7 +555,8 @@ public SerializableTransactionManager( ConflictTracer conflictTracer, MetricsFilterEvaluationContext metricsFilterEvaluationContext, Optional sharedGetRangesPoolSize, - TransactionKnowledgeComponents knowledge) { + TransactionKnowledgeComponents knowledge, + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager) { super( metricsManager, transactionKeyValueServiceManager, @@ -557,7 +580,8 @@ public SerializableTransactionManager( conflictTracer, metricsFilterEvaluationContext, sharedGetRangesPoolSize, - knowledge); + knowledge, + keyValueSnapshotReaderManager); this.conflictTracer = conflictTracer; } @@ -594,8 +618,10 @@ protected CallbackAwareTransaction createTransaction( conflictTracer, tableLevelMetricsController, knowledge, + keyValueSnapshotReaderManager, commitTimestampLoaderFactory.createCommitTimestampLoader( - startTimestampSupplier, immutableTimestamp, Optional.of(immutableTsLock))); + startTimestampSupplier, immutableTimestamp, Optional.of(immutableTsLock)), + createPreCommitConditionValidator(Optional.of(immutableTsLock), preCommitCondition)); } @VisibleForTesting diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SnapshotTransaction.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SnapshotTransaction.java index c2405db9ad9..8811c2677e2 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SnapshotTransaction.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SnapshotTransaction.java @@ -84,9 +84,13 @@ import com.palantir.atlasdb.transaction.api.ConflictHandler; import com.palantir.atlasdb.transaction.api.ConstraintCheckable; import com.palantir.atlasdb.transaction.api.ConstraintCheckingTransaction; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; import com.palantir.atlasdb.transaction.api.GetRangesQuery; import com.palantir.atlasdb.transaction.api.ImmutableGetRangesQuery; -import com.palantir.atlasdb.transaction.api.PreCommitCondition; +import com.palantir.atlasdb.transaction.api.ImmutableTransactionContext; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReader; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager; +import com.palantir.atlasdb.transaction.api.PreCommitRequirementValidator; import com.palantir.atlasdb.transaction.api.TransactionCommitFailedException; import com.palantir.atlasdb.transaction.api.TransactionConflictException; import com.palantir.atlasdb.transaction.api.TransactionConflictException.CellConflict; @@ -108,6 +112,7 @@ import com.palantir.atlasdb.transaction.impl.expectations.CellCountValidator; import com.palantir.atlasdb.transaction.impl.expectations.TrackingTransactionKeyValueService; import com.palantir.atlasdb.transaction.impl.expectations.TrackingTransactionKeyValueServiceImpl; +import com.palantir.atlasdb.transaction.impl.metrics.DefaultKeyValueSnapshotEventRecorder; import com.palantir.atlasdb.transaction.impl.metrics.TableLevelMetricsController; import com.palantir.atlasdb.transaction.impl.metrics.TransactionMetrics; import com.palantir.atlasdb.transaction.impl.metrics.TransactionOutcomeMetrics; @@ -243,6 +248,8 @@ private enum State { protected final TimelockService timelockService; protected final LockWatchManagerInternal lockWatchManager; final TrackingTransactionKeyValueService transactionKeyValueService; + protected final KeyValueSnapshotReaderManager keyValueSnapshotReaderManager; + final KeyValueSnapshotReader keyValueSnapshotReader; final TransactionService defaultTransactionService; private final AsyncTransactionService immediateTransactionService; private final Cleaner cleaner; @@ -254,7 +261,6 @@ private enum State { protected final long immutableTimestamp; protected final Optional immutableTimestampLock; - private final PreCommitCondition preCommitCondition; protected final long timeCreated = System.currentTimeMillis(); protected final LocalWriteBuffer localWriteBuffer = new LocalWriteBuffer(); @@ -299,13 +305,13 @@ private enum State { protected final TimestampCache timestampCache; protected final TransactionKnowledgeComponents knowledge; + protected final PreCommitRequirementValidator preCommitRequirementValidator; protected final Closer closer = Closer.create(); /** * @param immutableTimestamp If we find a row written before the immutableTimestamp we don't need to grab a read - * lock for it because we know that no writers exist. - * @param preCommitCondition This check must pass for this transaction to commit. + * lock for it because we know that no writers exist. */ /* package */ SnapshotTransaction( MetricsManager metricsManager, @@ -319,7 +325,6 @@ private enum State { SweepStrategyManager sweepStrategyManager, long immutableTimestamp, Optional immutableTimestampLock, - PreCommitCondition preCommitCondition, AtlasDbConstraintCheckingMode constraintCheckingMode, Long transactionTimeoutMillis, TransactionReadSentinelBehavior readSentinelBehavior, @@ -334,7 +339,9 @@ private enum State { ConflictTracer conflictTracer, TableLevelMetricsController tableLevelMetricsController, TransactionKnowledgeComponents knowledge, - CommitTimestampLoader commitTimestampLoader) { + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager, + CommitTimestampLoader commitTimestampLoader, + PreCommitRequirementValidator preCommitRequirementValidator) { this.metricsManager = metricsManager; this.lockWatchManager = lockWatchManager; this.conflictTracer = conflictTracer; @@ -350,7 +357,6 @@ private enum State { this.sweepStrategyManager = sweepStrategyManager; this.immutableTimestamp = immutableTimestamp; this.immutableTimestampLock = immutableTimestampLock; - this.preCommitCondition = preCommitCondition; this.constraintCheckingMode = constraintCheckingMode; this.transactionReadTimeoutMillis = transactionTimeoutMillis; this.readSentinelBehavior = readSentinelBehavior; @@ -371,6 +377,20 @@ private enum State { TransactionOutcomeMetrics.create(transactionMetrics, metricsManager.getTaggedRegistry()); this.expectationsDataCollectionMetrics = ExpectationsMetrics.of(metricsManager.getTaggedRegistry()); this.commitTimestampLoader = commitTimestampLoader; + this.preCommitRequirementValidator = preCommitRequirementValidator; + this.keyValueSnapshotReaderManager = keyValueSnapshotReaderManager; + this.keyValueSnapshotReader = getDefaultKeyValueSnapshotReader(); + } + + private KeyValueSnapshotReader getDefaultKeyValueSnapshotReader() { + return keyValueSnapshotReaderManager.createKeyValueSnapshotReader(ImmutableTransactionContext.builder() + .startTimestampSupplier(startTimestamp) + .transactionReadSentinelBehavior(TransactionReadSentinelBehavior.THROW_EXCEPTION) + .commitTimestampLoader(commitTimestampLoader) + .preCommitRequirementValidator(preCommitRequirementValidator) + .keyValueSnapshotEventRecorder( + DefaultKeyValueSnapshotEventRecorder.create(metricsManager, tableLevelMetricsController)) + .build()); } protected TransactionScopedCache getCache() { @@ -424,13 +444,7 @@ public NavigableMap> getRows( tableRef, rows, columnSelection, - cells -> AtlasFutures.getUnchecked(getInternal( - "getRows", - tableRef, - cells, - cells.size(), - transactionKeyValueService, - immediateTransactionService)), + cells -> AtlasFutures.getUnchecked(getInternal("getRows", tableRef, cells, cells.size())), unCachedRows -> getRowsInternal(tableRef, unCachedRows, columnSelection)); } @@ -442,20 +456,18 @@ private NavigableMap> getRowsInternal( return AbstractTransaction.EMPTY_SORTED_ROWS; } hasReads = true; - ImmutableSortedMap.Builder result = ImmutableSortedMap.naturalOrder(); - Map rawResults = - new HashMap<>(transactionKeyValueService.getRows(tableRef, rows, columnSelection, getStartTimestamp())); + + ImmutableSortedMap.Builder resultCollector = ImmutableSortedMap.naturalOrder(); NavigableMap writes = localWriteBuffer.getLocalWrites().get(tableRef); if (writes != null && !writes.isEmpty()) { for (byte[] row : rows) { - extractLocalWritesForRow(result, writes, row, columnSelection); + extractLocalWritesForRow(resultCollector, writes, row, columnSelection); } } - // We don't need to do work postFiltering if we have a write locally. - rawResults.keySet().removeAll(result.buildOrThrow().keySet()); + NavigableMap> results = + keyValueSnapshotReader.getRows(tableRef, rows, columnSelection, resultCollector); - NavigableMap> results = filterRowResults(tableRef, rawResults, result); long getRowsMillis = TimeUnit.NANOSECONDS.toMillis(timer.stop()); if (perfLogger.isDebugEnabled()) { perfLogger.debug( @@ -918,17 +930,7 @@ private void extractLocalWritesForRow( @Override @Idempotent public Map get(TableReference tableRef, Set cells) { - return getCache() - .get( - tableRef, - cells, - uncached -> getInternal( - "get", - tableRef, - uncached, - uncached.size(), - transactionKeyValueService, - immediateTransactionService)); + return getCache().get(tableRef, cells, uncached -> getInternal("get", tableRef, uncached, uncached.size())); } @Override @@ -944,9 +946,7 @@ public Result, MoreCellsPresentThanExpectedException> getWithE "getWithExpectedNumberOfCells", tableRef, cacheLookupResult.missedCells(), - numberOfCellsExpectingValuePostCache, - transactionKeyValueService, - immediateTransactionService); + numberOfCellsExpectingValuePostCache); })); } catch (MoreCellsPresentThanExpectedException e) { return Result.err(e); @@ -957,26 +957,12 @@ public Result, MoreCellsPresentThanExpectedException> getWithE @Idempotent public ListenableFuture> getAsync(TableReference tableRef, Set cells) { return scopeToTransaction(getCache() - .getAsync( - tableRef, - cells, - uncached -> getInternal( - "getAsync", - tableRef, - uncached, - uncached.size(), - transactionKeyValueService, - defaultTransactionService))); + .getAsync(tableRef, cells, uncached -> getInternal("getAsync", tableRef, uncached, uncached.size()))); } @VisibleForTesting ListenableFuture> getInternal( - String operationName, - TableReference tableRef, - Set cells, - long numberOfExpectedPresentCells, - TransactionKeyValueService asyncKeyValueService, - AsyncTransactionService asyncTransactionService) { + String operationName, TableReference tableRef, Set cells, long numberOfExpectedPresentCells) { Timer.Context timer = getTimer(operationName).time(); checkGetPreconditions(tableRef); if (Iterables.isEmpty(cells)) { @@ -1001,12 +987,12 @@ ListenableFuture> getInternal( // We don't need to read any cells that were written locally. long expectedNumberOfPresentCellsToFetch = numberOfExpectedPresentCells - numberOfNonDeleteLocalWrites; + + Set cellsToFetch = Sets.difference(cells, result.keySet()); + ListenableFuture> initialResults = keyValueSnapshotReader.getAsync(tableRef, cellsToFetch); + return Futures.transform( - getFromKeyValueService( - tableRef, - Sets.difference(cells, result.keySet()), - asyncKeyValueService, - asyncTransactionService), + initialResults, fromKeyValueService -> { result.putAll(fromKeyValueService); @@ -1040,10 +1026,7 @@ public Map getIgnoringLocalWrites(TableReference tableRef, Set> result = - getFromKeyValueService(tableRef, cells, transactionKeyValueService, immediateTransactionService); - - Map unfiltered = Futures.getUnchecked(result); + Map unfiltered = AtlasFutures.getUnchecked(keyValueSnapshotReader.getAsync(tableRef, cells)); Map filtered = Maps.filterValues(unfiltered, Predicates.not(Value::isTombstone)); TraceStatistics.incEmptyValues(unfiltered.size() - filtered.size()); @@ -1054,26 +1037,6 @@ public Map getIgnoringLocalWrites(TableReference tableRef, Set> getFromKeyValueService( - TableReference tableRef, - Set cells, - TransactionKeyValueService asyncKeyValueService, - AsyncTransactionService asyncTransactionService) { - Map toRead = Cells.constantValueMap(cells, getStartTimestamp()); - ListenableFuture>> postFilteredResults = Futures.transformAsync( - asyncKeyValueService.getAsync(tableRef, toRead), - rawResults -> getWithPostFilteringAsync( - tableRef, rawResults, Value.GET_VALUE, asyncKeyValueService, asyncTransactionService), - MoreExecutors.directExecutor()); - - return Futures.transform(postFilteredResults, ImmutableMap::copyOf, MoreExecutors.directExecutor()); - } - private static byte[] getNextStartRowName( RangeRequest range, TokenBackedBasicResultsPage, byte[]> prePostFilter) { if (!prePostFilter.moreResultsAvailable()) { @@ -1253,22 +1216,14 @@ private void validatePreCommitRequirementsOnNonExhaustiveReadIfNecessary(TableRe */ private void validatePreCommitRequirementsOnReadIfNecessary( TableReference tableRef, long timestamp, boolean allPossibleCellsReadAndPresent) { - if (isValidationNecessaryOnReads(tableRef, allPossibleCellsReadAndPresent)) { - throwIfPreCommitRequirementsNotMet(null, timestamp); - } else if (!allPossibleCellsReadAndPresent) { + boolean mayHaveUnvalidatedReads = preCommitRequirementValidator.throwIfPreCommitRequirementsNotMetOnRead( + tableRef, timestamp, allPossibleCellsReadAndPresent); + + if (mayHaveUnvalidatedReads) { hasPossiblyUnvalidatedReads = true; } } - private boolean isValidationNecessaryOnReads(TableReference tableRef, boolean allPossibleCellsReadAndPresent) { - return validateLocksOnReads && requiresImmutableTimestampLocking(tableRef, allPossibleCellsReadAndPresent); - } - - private boolean requiresImmutableTimestampLocking(TableReference tableRef, boolean allPossibleCellsReadAndPresent) { - return sweepStrategyManager.get(tableRef).mustCheckImmutableLock(allPossibleCellsReadAndPresent) - || transactionConfig.get().lockImmutableTsOnReadOnlyTransactions(); - } - private List> getPostFilteredWithLocalWrites( final TableReference tableRef, final SortedMap postFiltered, @@ -1434,7 +1389,7 @@ protected Iterator> computeNext() { /** * This includes deleted writes as zero length byte arrays, be sure to strip them out. - * + *

* For the selectedColumns parameter, empty set means all columns. This is unfortunate, but follows the semantics of * {@link RangeRequest}. */ @@ -1513,16 +1468,15 @@ private int estimateSize(List> rangeRows) { private Collection> getWithPostFilteringSync( TableReference tableRef, Map rawResults, Function transformer) { - return AtlasFutures.getUnchecked(getWithPostFilteringAsync( - tableRef, rawResults, transformer, transactionKeyValueService, immediateTransactionService)); + return AtlasFutures.getUnchecked( + getWithPostFilteringAsync(tableRef, rawResults, transformer, transactionKeyValueService)); } private ListenableFuture>> getWithPostFilteringAsync( TableReference tableRef, Map rawResults, Function transformer, - TransactionKeyValueService asyncKeyValueService, - AsyncTransactionService asyncTransactionService) { + TransactionKeyValueService asyncKeyValueService) { long bytes = 0; for (Map.Entry entry : rawResults.entrySet()) { bytes += entry.getValue().getContents().length + Cells.getApproxSizeOfCell(entry.getKey()); @@ -1562,12 +1516,7 @@ private ListenableFuture>> getWithPostFilterin return Futures.transformAsync( Futures.immediateFuture(rawResults), resultsToPostFilter -> getWithPostFilteringIterate( - tableRef, - resultsToPostFilter, - resultsAccumulator, - transformer, - asyncKeyValueService, - asyncTransactionService), + tableRef, resultsToPostFilter, resultsAccumulator, transformer, asyncKeyValueService), MoreExecutors.directExecutor()); } @@ -1576,8 +1525,7 @@ private ListenableFuture>> getWithPostFilterin Map resultsToPostFilter, Collection> resultsAccumulator, Function transformer, - TransactionKeyValueService asyncKeyValueService, - AsyncTransactionService asyncTransactionService) { + TransactionKeyValueService asyncKeyValueService) { return Futures.transformAsync( Futures.immediateFuture(resultsToPostFilter), results -> { @@ -1589,8 +1537,7 @@ private ListenableFuture>> getWithPostFilterin remainingResultsToPostFilter, resultsAccumulator, transformer, - asyncKeyValueService, - asyncTransactionService)); + asyncKeyValueService)); Preconditions.checkState( ++iterations < MAX_POST_FILTERING_ITERATIONS, "Unable to filter cells to find correct result after " @@ -1684,8 +1631,7 @@ private ListenableFuture> getWithPostFilteringInternal( Map rawResults, @Output Collection> resultsCollector, Function transformer, - TransactionKeyValueService asyncKeyValueService, - AsyncTransactionService asyncTransactionService) { + TransactionKeyValueService asyncKeyValueService) { Set orphanedSentinels = findOrphanedSweepSentinels(tableRef, rawResults); LongSet valuesStartTimestamps = getStartTimestampsForValues(rawResults.values()); @@ -1744,12 +1690,10 @@ private ListenableFuture> collectCellsToPostFilter( commitTimestamps.getIfAbsent(value.getTimestamp(), TransactionConstants.FAILED_COMMIT_TS); if (theirCommitTimestamp == TransactionConstants.FAILED_COMMIT_TS) { keysToReload.put(key, value.getTimestamp()); - if (shouldDeleteAndRollback()) { - // This is from a failed transaction so we can roll it back and then reload it. - keysToDelete.put(key, value.getTimestamp()); - getCounter(AtlasDbMetricNames.CellFilterMetrics.INVALID_COMMIT_TS, tableRef) - .inc(); - } + // This is from a failed transaction so we can roll it back and then reload it. + keysToDelete.put(key, value.getTimestamp()); + getCounter(AtlasDbMetricNames.CellFilterMetrics.INVALID_COMMIT_TS, tableRef) + .inc(); } else if (theirCommitTimestamp > getStartTimestamp()) { // The value's commit timestamp is after our start timestamp. // This means the value is from a transaction which committed @@ -1796,15 +1740,6 @@ private Map getRemainingResults(Map rawResults, Set cells) { deleteWithMetadataInternal(tableRef, cells, ImmutableMap.of()); @@ -1880,7 +1815,8 @@ public void abort() { ensureUncommitted(); if (state.compareAndSet(State.UNCOMMITTED, State.ABORTED)) { if (hasWrites()) { - throwIfPreCommitRequirementsNotMet(null, getStartTimestamp()); + preCommitRequirementValidator.throwIfPreCommitRequirementsNotMet( + Optional.empty(), getStartTimestamp()); } transactionOutcomeMetrics.markAbort(); if (transactionLengthLogger.isDebugEnabled()) { @@ -1937,7 +1873,7 @@ private boolean isStillRunning() { /** * Returns true iff the transaction is known to have successfully committed. - * + *

* Be careful when using this method! A transaction that the client thinks has failed could actually have * committed as far as the key-value service is concerned. */ @@ -2056,12 +1992,12 @@ private void commitWrites(TransactionService transactionService) { if (!hasWrites()) { if (hasReads() || hasAnyInvolvedTables()) { // verify any pre-commit conditions on the transaction - preCommitCondition.throwIfConditionInvalid(getStartTimestamp()); + preCommitRequirementValidator.throwIfPreCommitConditionInvalid(getStartTimestamp()); // if there are no writes, we must still make sure the immutable timestamp lock is still valid, // to ensure that sweep hasn't thoroughly deleted cells we tried to read if (validationNecessaryForInvolvedTablesOnCommit()) { - throwIfImmutableTsOrCommitLocksExpired(null); + preCommitRequirementValidator.throwIfImmutableTsOrCommitLocksExpired(Optional.empty()); } } getTimer("nonWriteCommitTotalTimeSinceTxCreation") @@ -2101,7 +2037,10 @@ private void commitWrites(TransactionService transactionService) { // Introduced for txn4 - Prevents sweep from making progress beyond immutableTs before entries were // put into the sweep queue. This ensures that sweep must process writes to the sweep queue done by // this transaction before making progress. - traced("postSweepEnqueueLockCheck", () -> throwIfImmutableTsOrCommitLocksExpired(commitLocksToken)); + traced( + "postSweepEnqueueLockCheck", + () -> preCommitRequirementValidator.throwIfImmutableTsOrCommitLocksExpired( + Optional.ofNullable(commitLocksToken))); // Write to the key value service. We must do this before getting the commit timestamp - otherwise // we risk another transaction starting at a timestamp after our commit timestamp not seeing our writes. @@ -2137,11 +2076,15 @@ private void commitWrites(TransactionService transactionService) { // handling - we should check lock validity last to ensure that sweep hasn't affected the checks. timedAndTraced( "userPreCommitCondition", - () -> throwIfPreCommitConditionInvalidAtCommitOnWriteTransaction(writes, commitTimestamp)); + () -> preCommitRequirementValidator.throwIfPreCommitConditionInvalidAtCommitOnWriteTransaction( + writes, commitTimestamp)); // Not timed, because this just calls ConjureTimelockServiceBlocking.refreshLockLeases, and that is // timed. - traced("preCommitLockCheck", () -> throwIfImmutableTsOrCommitLocksExpired(commitLocksToken)); + traced( + "preCommitLockCheck", + () -> preCommitRequirementValidator.throwIfImmutableTsOrCommitLocksExpired( + Optional.ofNullable(commitLocksToken))); // Not timed, because this just calls TransactionService.putUnlessExists, and that is timed. traced( @@ -2201,54 +2144,18 @@ protected ConflictHandler getConflictHandlerForTable(TableReference tableRef) { LoggingArgs.tableRef(tableRef))); } - private String getExpiredLocksErrorString(@Nullable LockToken commitLocksToken, Set expiredLocks) { + private String getExpiredLocksErrorString(Optional commitLocksToken, Set expiredLocks) { return "The following immutable timestamp lock was required: " + immutableTimestampLock + "; the following commit locks were required: " + commitLocksToken + "; the following locks are no longer valid: " + expiredLocks; } - private void throwIfPreCommitRequirementsNotMet(@Nullable LockToken commitLocksToken, long timestamp) { - throwIfImmutableTsOrCommitLocksExpired(commitLocksToken); - throwIfPreCommitConditionInvalid(timestamp); - } - - private void throwIfPreCommitConditionInvalid(long timestamp) { - try { - preCommitCondition.throwIfConditionInvalid(timestamp); - } catch (TransactionFailedException ex) { - transactionOutcomeMetrics.markPreCommitCheckFailed(); - throw ex; - } - } - - private void throwIfPreCommitConditionInvalidAtCommitOnWriteTransaction( - Map> mutations, long timestamp) { - try { - preCommitCondition.throwIfConditionInvalid(mutations, timestamp); - } catch (TransactionFailedException ex) { - transactionOutcomeMetrics.markPreCommitCheckFailed(); - throw ex; - } - } - - private void throwIfImmutableTsOrCommitLocksExpired(@Nullable LockToken commitLocksToken) { - Set expiredLocks = refreshCommitAndImmutableTsLocks(commitLocksToken); - if (!expiredLocks.isEmpty()) { - final String baseMsg = "Locks acquired as part of the transaction protocol are no longer valid. "; - String expiredLocksErrorString = getExpiredLocksErrorString(commitLocksToken, expiredLocks); - TransactionLockTimeoutException ex = new TransactionLockTimeoutException(baseMsg + expiredLocksErrorString); - log.warn(baseMsg + "{}", UnsafeArg.of("expiredLocksErrorString", expiredLocksErrorString), ex); - transactionOutcomeMetrics.markLocksExpired(); - throw ex; - } - } - /** * Refreshes external and commit locks. * * @return set of locks that could not be refreshed */ - private Set refreshCommitAndImmutableTsLocks(@Nullable LockToken commitLocksToken) { + private Set refreshCommitAndImmutableTsLocks(LockToken commitLocksToken) { Set toRefresh = new HashSet<>(); if (commitLocksToken != null) { toRefresh.add(commitLocksToken); @@ -2339,13 +2246,15 @@ private void throwIfValueChangedConflict( } if (!conflictingValues.containsKey(cell)) { // This error case could happen if our locks expired. - throwIfPreCommitRequirementsNotMet(commitLocksToken, getStartTimestamp()); + preCommitRequirementValidator.throwIfPreCommitRequirementsNotMet( + Optional.ofNullable(commitLocksToken), getStartTimestamp()); Validate.isTrue( false, "Missing conflicting value for cell: %s for table %s", cellToConflict.get(cell), table); } if (conflictingValues.get(cell).getTimestamp() != (cellEntry.getValue() - 1)) { // This error case could happen if our locks expired. - throwIfPreCommitRequirementsNotMet(commitLocksToken, getStartTimestamp()); + preCommitRequirementValidator.throwIfPreCommitRequirementsNotMet( + Optional.ofNullable(commitLocksToken), getStartTimestamp()); Validate.isTrue( false, "Wrong timestamp for cell in table %s Expected: %s Actual: %s", @@ -2666,7 +2575,7 @@ private LongLongMap getCommitTimestampsSync( /** * This will attempt to put the commitTimestamp into the DB. * - * @throws TransactionLockTimeoutException If our locks timed out while trying to commit. + * @throws TransactionLockTimeoutException If our locks timed out while trying to commit. * @throws TransactionCommitFailedException failed when committing in a way that isn't retriable */ private void putCommitTimestamp(long commitTimestamp, LockToken locksToken, TransactionService transactionService) @@ -2701,7 +2610,7 @@ private void handleKeyAlreadyExistsException( throw new TransactionLockTimeoutException( "Our commit was already rolled back at commit time" + " because our locks timed out. startTs: " + getStartTimestamp() + ". " - + getExpiredLocksErrorString(commitLocksToken, expiredLocks), + + getExpiredLocksErrorString(Optional.of(commitLocksToken), expiredLocks), ex); } else { log.info( @@ -2772,6 +2681,11 @@ private boolean validationNecessaryForInvolvedTablesOnCommit() { return anyTableRequiresImmutableTimestampLocking && needsToValidate; } + private boolean requiresImmutableTimestampLocking(TableReference tableRef, boolean allPossibleCellsReadAndPresent) { + return sweepStrategyManager.get(tableRef).mustCheckImmutableLock(allPossibleCellsReadAndPresent) + || transactionConfig.get().lockImmutableTsOnReadOnlyTransactions(); + } + @Override public long getAgeMillis() { return System.currentTimeMillis() - timeCreated; diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionManager.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionManager.java index 8526ae1f2ec..e0ba474968a 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionManager.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionManager.java @@ -38,7 +38,9 @@ import com.palantir.atlasdb.transaction.TransactionConfig; import com.palantir.atlasdb.transaction.api.AtlasDbConstraintCheckingMode; import com.palantir.atlasdb.transaction.api.ConditionAwareTransactionTask; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; import com.palantir.atlasdb.transaction.api.KeyValueServiceStatus; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager; import com.palantir.atlasdb.transaction.api.OpenTransaction; import com.palantir.atlasdb.transaction.api.PreCommitCondition; import com.palantir.atlasdb.transaction.api.Transaction; @@ -50,6 +52,8 @@ import com.palantir.atlasdb.transaction.impl.metrics.MetricsFilterEvaluationContext; import com.palantir.atlasdb.transaction.impl.metrics.TableLevelMetricsController; import com.palantir.atlasdb.transaction.impl.metrics.ToplistDeltaFilteringTableLevelMetricsController; +import com.palantir.atlasdb.transaction.impl.metrics.TransactionMetrics; +import com.palantir.atlasdb.transaction.impl.metrics.TransactionOutcomeMetrics; import com.palantir.atlasdb.transaction.knowledge.TransactionKnowledgeComponents; import com.palantir.atlasdb.transaction.service.TransactionService; import com.palantir.atlasdb.util.MetricsManager; @@ -112,6 +116,7 @@ private final ConflictTracer conflictTracer; protected final TransactionKnowledgeComponents knowledge; + protected final KeyValueSnapshotReaderManager keyValueSnapshotReaderManager; protected final CommitTimestampLoaderFactory commitTimestampLoaderFactory; protected SnapshotTransactionManager( @@ -137,7 +142,8 @@ protected SnapshotTransactionManager( ConflictTracer conflictTracer, MetricsFilterEvaluationContext metricsFilterEvaluationContext, Optional sharedGetRangesPoolSize, - TransactionKnowledgeComponents knowledge) { + TransactionKnowledgeComponents knowledge, + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager) { super(metricsManager, timestampCache, () -> transactionConfig.get().retryStrategy()); this.lockWatchManager = lockWatchManager; TimestampTracker.instrumentTimestamps(metricsManager, timelockService, cleaner); @@ -167,6 +173,7 @@ protected SnapshotTransactionManager( this.openTransactionCounter = metricsManager.registerOrGetCounter(SnapshotTransactionManager.class, "openTransactionCounter"); this.knowledge = knowledge; + this.keyValueSnapshotReaderManager = keyValueSnapshotReaderManager; this.commitTimestampLoaderFactory = new CommitTimestampLoaderFactory( timestampCache, metricsManager, timelockService, knowledge, transactionService, transactionConfig); } @@ -321,7 +328,6 @@ protected CallbackAwareTransaction createTransaction( sweepStrategyManager, immutableTimestamp, immutableTimestampLock, - condition, constraintModeSupplier.get(), cleaner.getTransactionReadTimeoutMillis(), TransactionReadSentinelBehavior.THROW_EXCEPTION, @@ -336,8 +342,23 @@ protected CallbackAwareTransaction createTransaction( conflictTracer, tableLevelMetricsController, knowledge, + keyValueSnapshotReaderManager, commitTimestampLoaderFactory.createCommitTimestampLoader( - startTimestampSupplier, immutableTimestamp, immutableTimestampLock)); + startTimestampSupplier, immutableTimestamp, immutableTimestampLock), + createPreCommitConditionValidator(immutableTimestampLock, condition)); + } + + protected final DefaultPreCommitRequirementValidator createPreCommitConditionValidator( + Optional immutableTsLock, PreCommitCondition condition) { + return new DefaultPreCommitRequirementValidator( + condition, + sweepStrategyManager, + transactionConfig, + new DefaultLockRefresher(timelockService), + immutableTsLock, + validateLocksOnReads, + TransactionOutcomeMetrics.create( + TransactionMetrics.of(metricsManager.getTaggedRegistry()), metricsManager.getTaggedRegistry())); } @Override @@ -356,6 +377,7 @@ private T runTaskWithCond long immutableTs = getApproximateImmutableTimestamp(); LongSupplier startTimestampSupplier = getStartTimestampSupplier(); Optional immutableTimestampLock = Optional.empty(); + SnapshotTransaction transaction = new SnapshotTransaction( metricsManager, transactionKeyValueServiceManager.getTransactionKeyValueService(startTimestampSupplier), @@ -368,7 +390,6 @@ private T runTaskWithCond sweepStrategyManager, immutableTs, immutableTimestampLock, - condition, constraintModeSupplier.get(), cleaner.getTransactionReadTimeoutMillis(), TransactionReadSentinelBehavior.THROW_EXCEPTION, @@ -383,8 +404,10 @@ private T runTaskWithCond conflictTracer, tableLevelMetricsController, knowledge, + keyValueSnapshotReaderManager, commitTimestampLoaderFactory.createCommitTimestampLoader( - startTimestampSupplier, immutableTs, immutableTimestampLock)); + startTimestampSupplier, immutableTs, immutableTimestampLock), + createPreCommitConditionValidator(immutableTimestampLock, condition)); return runTaskThrowOnConflictWithCallback( txn -> task.execute(txn, condition), new ReadTransaction(transaction, sweepStrategyManager), @@ -400,7 +423,7 @@ public void registerClosingCallback(Runnable closingCallback) { /** * Frees resources used by this SnapshotTransactionManager, and invokes any callbacks registered to run on close. * This includes the cleaner, the key value service (and attendant thread pools), and possibly the lock service. - * + *

* Concurrency: If this method races with registerClosingCallback(closingCallback), then closingCallback * may be called (but is not necessarily called). Callbacks registered before the invocation of close() are * guaranteed to be executed (because we use a synchronized list) as long as no exceptions arise. If an exception diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/metrics/DefaultKeyValueSnapshotEventRecorder.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/metrics/DefaultKeyValueSnapshotEventRecorder.java new file mode 100644 index 00000000000..bb5edf9ccc2 --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/metrics/DefaultKeyValueSnapshotEventRecorder.java @@ -0,0 +1,96 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl.metrics; + +import com.palantir.atlasdb.AtlasDbMetricNames; +import com.palantir.atlasdb.AtlasDbMetricNames.CellFilterMetrics; +import com.palantir.atlasdb.keyvalue.api.TableReference; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotEventRecorder; +import com.palantir.atlasdb.util.MetricsManager; + +public final class DefaultKeyValueSnapshotEventRecorder implements KeyValueSnapshotEventRecorder { + // This dichotomy is unfortunate, but a result of us standing between the legacy and metric-schema worlds. + private final SnapshotTransactionMetricFactory metricFactory; + private final TransactionMetrics transactionMetrics; + + private DefaultKeyValueSnapshotEventRecorder( + SnapshotTransactionMetricFactory metricFactory, TransactionMetrics transactionMetrics) { + this.metricFactory = metricFactory; + this.transactionMetrics = transactionMetrics; + } + + public static KeyValueSnapshotEventRecorder create( + MetricsManager metricsManager, TableLevelMetricsController tableLevelMetricsController) { + return new DefaultKeyValueSnapshotEventRecorder( + new SnapshotTransactionMetricFactory(metricsManager, tableLevelMetricsController), + TransactionMetrics.of(metricsManager.getTaggedRegistry())); + } + + @Override + public void recordCellsRead(TableReference tableReference, long cellsRead) { + metricFactory + .getCounter(AtlasDbMetricNames.SNAPSHOT_TRANSACTION_CELLS_READ, tableReference) + .inc(cellsRead); + } + + @Override + public void recordCellsReturned(TableReference tableReference, long cellsReturned) { + metricFactory + .getCounter(AtlasDbMetricNames.SNAPSHOT_TRANSACTION_CELLS_RETURNED, tableReference) + .inc(cellsReturned); + } + + @Override + public void recordManyBytesReadForTable(TableReference tableReference, long bytesRead) { + metricFactory + .getHistogram(AtlasDbMetricNames.SNAPSHOT_TRANSACTION_TOO_MANY_BYTES_READ, tableReference) + .update(bytesRead); + } + + @Override + public void recordFilteredSweepSentinel(TableReference tableReference) { + metricFactory + .getCounter(CellFilterMetrics.INVALID_START_TS, tableReference) + .inc(); + } + + @Override + public void recordFilteredUncommittedTransaction(TableReference tableReference) { + metricFactory + .getCounter(CellFilterMetrics.INVALID_COMMIT_TS, tableReference) + .inc(); + } + + @Override + public void recordFilteredTransactionCommittingAfterOurStart(TableReference tableReference) { + metricFactory + .getCounter(CellFilterMetrics.COMMIT_TS_GREATER_THAN_TRANSACTION_TS, tableReference) + .inc(); + } + + @Override + public void recordRolledBackOtherTransaction() { + transactionMetrics.rolledBackOtherTransaction().mark(); + } + + @Override + public void recordEmptyValueRead(TableReference tableReference) { + metricFactory + .getCounter(AtlasDbMetricNames.CellFilterMetrics.EMPTY_VALUE, tableReference) + .inc(); + } +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/metrics/SnapshotTransactionMetricFactory.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/metrics/SnapshotTransactionMetricFactory.java new file mode 100644 index 00000000000..75dde405a01 --- /dev/null +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/transaction/impl/metrics/SnapshotTransactionMetricFactory.java @@ -0,0 +1,44 @@ +/* + * (c) Copyright 2024 Palantir Technologies Inc. All rights reserved. + * + * Licensed 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 com.palantir.atlasdb.transaction.impl.metrics; + +import com.codahale.metrics.Counter; +import com.codahale.metrics.Histogram; +import com.palantir.atlasdb.keyvalue.api.TableReference; +import com.palantir.atlasdb.transaction.impl.SnapshotTransaction; +import com.palantir.atlasdb.util.MetricsManager; + +public final class SnapshotTransactionMetricFactory { + private final MetricsManager metricsManager; + private final TableLevelMetricsController tableLevelMetricsController; + + public SnapshotTransactionMetricFactory( + MetricsManager metricsManager, TableLevelMetricsController tableLevelMetricsController) { + this.metricsManager = metricsManager; + this.tableLevelMetricsController = tableLevelMetricsController; + } + + // Using the SnapshotTransaction class and not this class, to preserve backwards compatibility + public Histogram getHistogram(String name, TableReference tableRef) { + return metricsManager.registerOrGetTaggedHistogram( + SnapshotTransaction.class, name, metricsManager.getTableNameTagFor(tableRef)); + } + + public Counter getCounter(String name, TableReference tableRef) { + return tableLevelMetricsController.createAndRegisterCounter(SnapshotTransaction.class, name, tableRef); + } +} diff --git a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/util/ByteArrayUtilities.java b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/util/ByteArrayUtilities.java index 4ce795a4c1e..678ec084962 100644 --- a/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/util/ByteArrayUtilities.java +++ b/atlasdb-impl-shared/src/main/java/com/palantir/atlasdb/util/ByteArrayUtilities.java @@ -46,7 +46,7 @@ public static boolean areMapsEqual(Map map1, Map map return true; } - private static boolean areByteMapsEqual(SortedMap map1, SortedMap map2) { + public static boolean areByteMapsEqual(SortedMap map1, SortedMap map2) { if (map1.size() != map2.size()) { return false; } diff --git a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/AbstractSerializableTransactionTest.java b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/AbstractSerializableTransactionTest.java index 07b9b8c551b..12270e482cf 100644 --- a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/AbstractSerializableTransactionTest.java +++ b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/AbstractSerializableTransactionTest.java @@ -41,7 +41,6 @@ import com.google.common.collect.Streams; import com.google.common.primitives.UnsignedBytes; import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.MoreExecutors; import com.palantir.atlasdb.AtlasDbConstants; import com.palantir.atlasdb.cleaner.NoOpCleaner; import com.palantir.atlasdb.debug.ConflictTracer; @@ -59,6 +58,7 @@ import com.palantir.atlasdb.keyvalue.impl.TransactionManagerManager; import com.palantir.atlasdb.sweep.queue.MultiTableSweepQueueWriter; import com.palantir.atlasdb.table.description.ValueType; +import com.palantir.atlasdb.transaction.ImmutableTransactionConfig; import com.palantir.atlasdb.transaction.api.AtlasDbConstraintCheckingMode; import com.palantir.atlasdb.transaction.api.ConflictHandler; import com.palantir.atlasdb.transaction.api.PreCommitCondition; @@ -71,6 +71,8 @@ import com.palantir.atlasdb.transaction.api.TransactionSerializableConflictException; import com.palantir.atlasdb.transaction.impl.SerializableTransaction.CellLoader; import com.palantir.atlasdb.transaction.impl.metrics.SimpleTableLevelMetricsController; +import com.palantir.atlasdb.transaction.impl.metrics.TransactionMetrics; +import com.palantir.atlasdb.transaction.impl.metrics.TransactionOutcomeMetrics; import com.palantir.atlasdb.util.MetricsManagers; import com.palantir.common.base.BatchingVisitable; import com.palantir.common.base.BatchingVisitables; @@ -172,16 +174,16 @@ private Transaction startTransactionWithOptions(TransactionOptions options) { AbstractTransactionTest.GET_RANGES_EXECUTOR, AbstractTransactionTest.DEFAULT_GET_RANGES_CONCURRENCY, getSweepQueueWriterInitialized(), - new DefaultDeleteExecutor( - transactionKeyValueServiceManager.getKeyValueService().orElseThrow(), - MoreExecutors.newDirectExecutorService()), + deleteExecutor, true, transactionConfigSupplier, ConflictTracer.NO_OP, new SimpleTableLevelMetricsController(metricsManager), knowledge, + keyValueSnapshotReaderManager, commitTimestampLoaderFactory.createCommitTimestampLoader( - startTimestampSupplier, 0L, options.immutableLockToken)) { + startTimestampSupplier, 0L, options.immutableLockToken), + createPreCommitConditionValidator(options.immutableLockToken, options.condition)) { @Override protected Map transformGetsForTesting(Map map) { return Maps.transformValues(map, byte[]::clone); @@ -1676,4 +1678,17 @@ private List generateColumns(int colCount) { .mapToObj(idx -> PtBytes.toBytes(getColumnWithIndex(idx))) .collect(Collectors.toList()); } + + private DefaultPreCommitRequirementValidator createPreCommitConditionValidator( + Optional immutableTsLock, PreCommitCondition condition) { + return new DefaultPreCommitRequirementValidator( + condition, + sweepStrategyManager, + () -> ImmutableTransactionConfig.builder().build(), + new DefaultLockRefresher(timelockService), + immutableTsLock, + true, + TransactionOutcomeMetrics.create( + TransactionMetrics.of(metricsManager.getTaggedRegistry()), metricsManager.getTaggedRegistry())); + } } diff --git a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManagerImpl.java b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManagerImpl.java index aaf4ad92643..576aacc53a8 100644 --- a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManagerImpl.java +++ b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TestTransactionManagerImpl.java @@ -31,6 +31,7 @@ import com.palantir.atlasdb.transaction.TransactionConfig; import com.palantir.atlasdb.transaction.api.AtlasDbConstraintCheckingMode; import com.palantir.atlasdb.transaction.api.ConflictHandler; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager; import com.palantir.atlasdb.transaction.api.PreCommitCondition; import com.palantir.atlasdb.transaction.api.Transaction; import com.palantir.atlasdb.transaction.api.TransactionReadSentinelBehavior; @@ -66,7 +67,8 @@ public TestTransactionManagerImpl( TimestampCache timestampCache, MultiTableSweepQueueWriter sweepQueue, TransactionKnowledgeComponents knowledge, - ExecutorService deleteExecutor) { + ExecutorService deleteExecutor, + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager) { this( metricsManager, keyValueService, @@ -79,7 +81,8 @@ public TestTransactionManagerImpl( sweepQueue, deleteExecutor, WrapperWithTracker.TRANSACTION_NO_OP, - knowledge); + knowledge, + keyValueSnapshotReaderManager); } public TestTransactionManagerImpl( @@ -94,7 +97,8 @@ public TestTransactionManagerImpl( MultiTableSweepQueueWriter sweepQueue, ExecutorService deleteExecutor, WrapperWithTracker transactionWrapper, - TransactionKnowledgeComponents knowledge) { + TransactionKnowledgeComponents knowledge, + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager) { this( metricsManager, createAssertKeyValue(keyValueService, lockService), @@ -107,7 +111,8 @@ public TestTransactionManagerImpl( sweepQueue, deleteExecutor, transactionWrapper, - knowledge); + knowledge, + keyValueSnapshotReaderManager); } private TestTransactionManagerImpl( @@ -122,7 +127,8 @@ private TestTransactionManagerImpl( MultiTableSweepQueueWriter sweepQueue, ExecutorService deleteExecutor, WrapperWithTracker transactionWrapper, - TransactionKnowledgeComponents knowledge) { + TransactionKnowledgeComponents knowledge, + KeyValueSnapshotReaderManager keyValueSnapshotReaderManager) { super( metricsManager, keyValueService, @@ -146,7 +152,8 @@ private TestTransactionManagerImpl( ConflictTracer.NO_OP, DefaultMetricsFilterEvaluationContext.createDefault(), Optional.empty(), - knowledge); + knowledge, + keyValueSnapshotReaderManager); this.transactionWrapper = transactionWrapper; } @@ -177,6 +184,7 @@ protected CallbackAwareTransaction createTransaction( LockToken immutableTsLock, PreCommitCondition preCommitCondition) { PathTypeTracker pathTypeTracker = PathTypeTrackers.constructSynchronousTracker(); + return transactionWrapper.apply( new SerializableTransaction( metricsManager, @@ -205,8 +213,10 @@ protected CallbackAwareTransaction createTransaction( ConflictTracer.NO_OP, tableLevelMetricsController, knowledge, + keyValueSnapshotReaderManager, commitTimestampLoaderFactory.createCommitTimestampLoader( - startTimestampSupplier, immutableTimestamp, Optional.of(immutableTsLock))), + startTimestampSupplier, immutableTimestamp, Optional.of(immutableTsLock)), + createPreCommitConditionValidator(Optional.of(immutableTsLock), preCommitCondition)), pathTypeTracker); } diff --git a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TransactionTestSetup.java b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TransactionTestSetup.java index b35d170ca23..1896f636121 100644 --- a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TransactionTestSetup.java +++ b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/transaction/impl/TransactionTestSetup.java @@ -47,6 +47,8 @@ import com.palantir.atlasdb.transaction.ImmutableTransactionConfig; import com.palantir.atlasdb.transaction.TransactionConfig; import com.palantir.atlasdb.transaction.api.ConflictHandler; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager; import com.palantir.atlasdb.transaction.api.Transaction; import com.palantir.atlasdb.transaction.api.TransactionManager; import com.palantir.atlasdb.transaction.knowledge.TransactionKnowledgeComponents; @@ -125,12 +127,14 @@ public static void storageTearDown() throws Exception { protected ConflictDetectionManager conflictDetectionManager; protected SweepStrategyManager sweepStrategyManager; protected TransactionManager txMgr; + protected DeleteExecutor deleteExecutor; protected TimestampCache timestampCache; protected TransactionKnowledgeComponents knowledge; protected Supplier transactionConfigSupplier; protected CommitTimestampLoaderFactory commitTimestampLoaderFactory; + protected KeyValueSnapshotReaderManager keyValueSnapshotReaderManager; @RegisterExtension public InMemoryTimelockExtension inMemoryTimelockExtension = new InMemoryTimelockExtension(); @@ -150,6 +154,9 @@ public void setUp() { keyValueService = getKeyValueService(); transactionKeyValueServiceManager = new DelegatingTransactionKeyValueServiceManager(keyValueService); + deleteExecutor = new DefaultDeleteExecutor( + transactionKeyValueServiceManager.getKeyValueService().orElseThrow(), + MoreExecutors.newDirectExecutorService()); keyValueService.createTables(ImmutableMap.of( TEST_TABLE_THOROUGH, TableMetadata.builder() @@ -205,6 +212,12 @@ public void setUp() { transactionConfigSupplier); conflictDetectionManager = ConflictDetectionManagers.createWithoutWarmingCache(keyValueService); sweepStrategyManager = SweepStrategyManagers.createDefault(keyValueService); + keyValueSnapshotReaderManager = new DefaultKeyValueSnapshotReaderManager( + transactionKeyValueServiceManager, + transactionService, + false, + new DefaultOrphanedSentinelDeleter(sweepStrategyManager::get, deleteExecutor), + deleteExecutor); txMgr = createAndRegisterManager(); } @@ -234,7 +247,8 @@ protected TransactionManager createManager() { timestampCache, MultiTableSweepQueueWriter.NO_OP, knowledge, - MoreExecutors.newDirectExecutorService()); + MoreExecutors.newDirectExecutorService(), + keyValueSnapshotReaderManager); } protected void put(Transaction txn, String rowName, String columnName, String value) { diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/AtlasDbTestCase.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/AtlasDbTestCase.java index 42fa2266f9d..c7b0e6fd66a 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/AtlasDbTestCase.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/AtlasDbTestCase.java @@ -30,9 +30,14 @@ import com.palantir.atlasdb.sweep.queue.SpecialTimestampsSupplier; import com.palantir.atlasdb.sweep.queue.TargetedSweeper; import com.palantir.atlasdb.transaction.api.ConflictHandler; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager; import com.palantir.atlasdb.transaction.impl.CachingTestTransactionManager; import com.palantir.atlasdb.transaction.impl.ConflictDetectionManager; import com.palantir.atlasdb.transaction.impl.ConflictDetectionManagers; +import com.palantir.atlasdb.transaction.impl.DefaultDeleteExecutor; +import com.palantir.atlasdb.transaction.impl.DefaultKeyValueSnapshotReaderManager; +import com.palantir.atlasdb.transaction.impl.DefaultOrphanedSentinelDeleter; import com.palantir.atlasdb.transaction.impl.SweepStrategyManager; import com.palantir.atlasdb.transaction.impl.SweepStrategyManagers; import com.palantir.atlasdb.transaction.impl.TestTransactionManager; @@ -76,6 +81,7 @@ public class AtlasDbTestCase { protected int sweepQueueShards = 128; protected TransactionKnowledgeComponents knowledge; + protected KeyValueSnapshotReaderManager keyValueSnapshotReaderManager; protected ExecutorService deleteExecutor; @@ -97,6 +103,13 @@ public void setUp() throws Exception { sweepStrategyManager = SweepStrategyManagers.createDefault(keyValueService); sweepQueue = spy(TargetedSweeper.createUninitializedForTest(keyValueService, () -> sweepQueueShards)); knowledge = TransactionKnowledgeComponents.createForTests(keyValueService, metricsManager.getTaggedRegistry()); + DeleteExecutor typedDeleteExecutor = new DefaultDeleteExecutor(keyValueService, deleteExecutor); + keyValueSnapshotReaderManager = new DefaultKeyValueSnapshotReaderManager( + txnKeyValueServiceManager, + transactionService, + false, + new DefaultOrphanedSentinelDeleter(sweepStrategyManager::get, typedDeleteExecutor), + typedDeleteExecutor); setUpTransactionManagers(); sweepQueue.initialize(serializableTxManager); sweepTimestampSupplier = new SpecialTimestampsSupplier( @@ -125,7 +138,8 @@ protected TestTransactionManager constructTestTransactionManager() { DefaultTimestampCache.createForTests(), sweepQueue, knowledge, - MoreExecutors.newDirectExecutorService()); + MoreExecutors.newDirectExecutorService(), + keyValueSnapshotReaderManager); } protected KeyValueService getBaseKeyValueService() { diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/schema/TableMigratorTest.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/schema/TableMigratorTest.java index 367f2fe438f..56e233ea9f5 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/schema/TableMigratorTest.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/schema/TableMigratorTest.java @@ -30,13 +30,18 @@ import com.palantir.atlasdb.keyvalue.api.RangeRequest; import com.palantir.atlasdb.keyvalue.api.RowResult; import com.palantir.atlasdb.keyvalue.api.TableReference; +import com.palantir.atlasdb.keyvalue.impl.DelegatingTransactionKeyValueServiceManager; import com.palantir.atlasdb.keyvalue.impl.InMemoryKeyValueService; import com.palantir.atlasdb.sweep.queue.MultiTableSweepQueueWriter; import com.palantir.atlasdb.table.description.TableDefinition; import com.palantir.atlasdb.table.description.ValueType; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; import com.palantir.atlasdb.transaction.api.TransactionTask; import com.palantir.atlasdb.transaction.impl.ConflictDetectionManager; import com.palantir.atlasdb.transaction.impl.ConflictDetectionManagers; +import com.palantir.atlasdb.transaction.impl.DefaultDeleteExecutor; +import com.palantir.atlasdb.transaction.impl.DefaultKeyValueSnapshotReaderManager; +import com.palantir.atlasdb.transaction.impl.DefaultOrphanedSentinelDeleter; import com.palantir.atlasdb.transaction.impl.SweepStrategyManager; import com.palantir.atlasdb.transaction.impl.SweepStrategyManagers; import com.palantir.atlasdb.transaction.impl.TestTransactionManagerImpl; @@ -47,6 +52,7 @@ import com.palantir.common.base.BatchingVisitable; import com.palantir.common.concurrent.PTExecutors; import java.util.Map; +import java.util.concurrent.ExecutorService; import org.apache.commons.lang3.mutable.MutableLong; import org.junit.jupiter.api.Test; @@ -90,6 +96,8 @@ public void testMigrationToDifferentKvs() { final ConflictDetectionManager cdm2 = ConflictDetectionManagers.createWithNoConflictDetection(); final SweepStrategyManager ssm2 = SweepStrategyManagers.completelyConservative(kvs2); final MetricsManager metricsManager = MetricsManagers.createForTests(); + final ExecutorService deleteExecutor2 = MoreExecutors.newDirectExecutorService(); + final DeleteExecutor typedDeleteExecutor2 = new DefaultDeleteExecutor(kvs2, deleteExecutor2); final TestTransactionManagerImpl txManager2 = new TestTransactionManagerImpl( metricsManager, kvs2, @@ -101,7 +109,13 @@ public void testMigrationToDifferentKvs() { DefaultTimestampCache.createForTests(), MultiTableSweepQueueWriter.NO_OP, TransactionKnowledgeComponents.createForTests(kvs2, metricsManager.getTaggedRegistry()), - MoreExecutors.newDirectExecutorService()); + deleteExecutor2, + new DefaultKeyValueSnapshotReaderManager( + new DelegatingTransactionKeyValueServiceManager(kvs2), + transactionService, + false, + new DefaultOrphanedSentinelDeleter(ssm2::get, typedDeleteExecutor2), + typedDeleteExecutor2)); kvs2.createTable(tableRef, definition.toTableMetadata().persistToBytes()); kvs2.createTable(namespacedTableRef, definition.toTableMetadata().persistToBytes()); @@ -128,6 +142,8 @@ public void testMigrationToDifferentKvs() { final ConflictDetectionManager verifyCdm = ConflictDetectionManagers.createWithNoConflictDetection(); final SweepStrategyManager verifySsm = SweepStrategyManagers.completelyConservative(kvs2); + final ExecutorService verifyDeleteExecutor = MoreExecutors.newDirectExecutorService(); + final DeleteExecutor verifyTypedDeleteExecutor = new DefaultDeleteExecutor(kvs2, verifyDeleteExecutor); final TestTransactionManagerImpl verifyTxManager = new TestTransactionManagerImpl( metricsManager, kvs2, @@ -139,7 +155,13 @@ public void testMigrationToDifferentKvs() { DefaultTimestampCache.createForTests(), MultiTableSweepQueueWriter.NO_OP, TransactionKnowledgeComponents.createForTests(kvs2, metricsManager.getTaggedRegistry()), - MoreExecutors.newDirectExecutorService()); + verifyDeleteExecutor, + new DefaultKeyValueSnapshotReaderManager( + new DelegatingTransactionKeyValueServiceManager(kvs2), + transactionService, + false, + new DefaultOrphanedSentinelDeleter(verifySsm::get, verifyTypedDeleteExecutor), + verifyTypedDeleteExecutor)); final MutableLong count = new MutableLong(); for (final TableReference name : Lists.newArrayList(tableRef, namespacedTableRef)) { verifyTxManager.runTaskReadOnly((TransactionTask) txn -> { diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/AbstractSnapshotTransactionTest.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/AbstractSnapshotTransactionTest.java index 00be6feeadf..3f88d4bc855 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/AbstractSnapshotTransactionTest.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/AbstractSnapshotTransactionTest.java @@ -56,7 +56,9 @@ import com.palantir.atlasdb.AtlasDbTestCase; import com.palantir.atlasdb.cache.DefaultTimestampCache; import com.palantir.atlasdb.cache.TimestampCache; +import com.palantir.atlasdb.cell.api.DdlManager; import com.palantir.atlasdb.cell.api.TransactionKeyValueService; +import com.palantir.atlasdb.cell.api.TransactionKeyValueServiceManager; import com.palantir.atlasdb.cleaner.NoOpCleaner; import com.palantir.atlasdb.debug.ConflictTracer; import com.palantir.atlasdb.encoding.PtBytes; @@ -82,6 +84,7 @@ import com.palantir.atlasdb.keyvalue.api.watch.LockWatchManagerInternal; import com.palantir.atlasdb.keyvalue.api.watch.LocksAndMetadata; import com.palantir.atlasdb.keyvalue.api.watch.NoOpLockWatchManager; +import com.palantir.atlasdb.keyvalue.impl.DelegatingTransactionKeyValueServiceManager; import com.palantir.atlasdb.logging.LoggingArgs; import com.palantir.atlasdb.protos.generated.TableMetadataPersistence.SweepStrategy; import com.palantir.atlasdb.ptobject.EncodingUtils; @@ -96,7 +99,9 @@ import com.palantir.atlasdb.transaction.api.ConflictHandler; import com.palantir.atlasdb.transaction.api.ConstraintCheckable; import com.palantir.atlasdb.transaction.api.ConstraintCheckingTransaction; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; import com.palantir.atlasdb.transaction.api.ImmutableGetRangesQuery; +import com.palantir.atlasdb.transaction.api.KeyValueSnapshotReaderManager; import com.palantir.atlasdb.transaction.api.LockAwareTransactionTask; import com.palantir.atlasdb.transaction.api.PreCommitCondition; import com.palantir.atlasdb.transaction.api.Transaction; @@ -212,6 +217,16 @@ public abstract class AbstractSnapshotTransactionTest extends AtlasDbTestCase { private final String name; private final WrapperWithTracker transactionWrapper; + // What? We'll remove this when productionising... + private final Map expectationsMapping = + ImmutableMap.builder() + .put(SYNC, AbstractSnapshotTransactionTest.this::asyncGetExpectation) + .put(ASYNC, AbstractSnapshotTransactionTest.this::asyncGetExpectation) + .buildOrThrow(); + + // WTF? But there's already a generic ExecutorService called deleteExecutor in the parent class... + private final DeleteExecutor typedDeleteExecutor = new DefaultDeleteExecutor(keyValueService, deleteExecutor); + private final TimestampCache timestampCache = new DefaultTimestampCache( metricsManager.getRegistry(), () -> AtlasDbConstants.DEFAULT_TIMESTAMP_CACHE_SIZE); private final ExecutorService getRangesExecutor = Executors.newFixedThreadPool(8); @@ -371,7 +386,8 @@ protected TestTransactionManager constructTestTransactionManager() { sweepQueue, deleteExecutor, transactionWrapper, - knowledge); + knowledge, + keyValueSnapshotReaderManager); } @Test @@ -439,6 +455,37 @@ public void testLockAfterGet() throws Exception { }); PathTypeTracker pathTypeTracker = PathTypeTrackers.constructSynchronousTracker(); + + KeyValueSnapshotReaderManager manager = new DefaultKeyValueSnapshotReaderManager( + new TransactionKeyValueServiceManager() { + @Override + public TransactionKeyValueService getTransactionKeyValueService(LongSupplier timestampSupplier) { + return tkvsMock; + } + + @Override + public Optional getKeyValueService() { + return Optional.empty(); + } + + @Override + public DdlManager getDdlManager() { + throw new SafeIllegalStateException("Not expecting to call in to DDL manager here"); + } + + @Override + public boolean isInitialized() { + return true; + } + + @Override + public void close() {} + }, + transactionService, + false, + new DefaultOrphanedSentinelDeleter(sweepStrategyManager::get, typedDeleteExecutor), + typedDeleteExecutor); + Transaction snapshot = transactionWrapper.apply( new SnapshotTransaction( metricsManager, @@ -452,7 +499,6 @@ public void testLockAfterGet() throws Exception { SweepStrategyManagers.createDefault(keyValueService), transactionTs, Optional.empty(), - PreCommitConditions.NO_OP, AtlasDbConstraintCheckingMode.NO_CONSTRAINT_CHECKING, null, TransactionReadSentinelBehavior.THROW_EXCEPTION, @@ -461,16 +507,17 @@ public void testLockAfterGet() throws Exception { getRangesExecutor, defaultGetRangesConcurrency, MultiTableSweepQueueWriter.NO_OP, - new DefaultDeleteExecutor( - txnKeyValueServiceManager.getKeyValueService().orElseThrow(), - MoreExecutors.newDirectExecutorService()), + typedDeleteExecutor, true, transactionConfig::get, ConflictTracer.NO_OP, tableLevelMetricsController, knowledge, + manager, // Maybe not createCommitTimestampLoader( - transactionTs, () -> transactionTs, Optional.empty(), timelockService)), + transactionTs, () -> transactionTs, Optional.empty(), timelockService), + createPreCommitConditionValidator( + Optional.empty(), PreCommitConditions.NO_OP, timelockService, true)), pathTypeTracker); assertThatThrownBy(() -> snapshot.get(TABLE, ImmutableSet.of(cell))).isInstanceOf(RuntimeException.class); @@ -501,7 +548,13 @@ public void testTransactionAtomicity() throws Exception { sweepQueue, MoreExecutors.newDirectExecutorService(), transactionWrapper, - knowledge); + knowledge, + new DefaultKeyValueSnapshotReaderManager( + new DelegatingTransactionKeyValueServiceManager(unstableKvs), + transactionService, + false, + new DefaultOrphanedSentinelDeleter(sweepStrategyManager::get, typedDeleteExecutor), + typedDeleteExecutor)); ScheduledExecutorService service = PTExecutors.newScheduledThreadPool(20); @@ -1020,6 +1073,7 @@ public void commitWithPreCommitConditionOnRetry() { @Test public void transactionDeletesAsyncOnRollback() { DeterministicScheduler executor = new DeterministicScheduler(); + DeleteExecutor typedDeleteExecutor = new DefaultDeleteExecutor(keyValueService, executor); TestTransactionManager deleteTxManager = new TestTransactionManagerImpl( metricsManager, keyValueService, @@ -1032,7 +1086,13 @@ public void transactionDeletesAsyncOnRollback() { sweepQueue, executor, transactionWrapper, - knowledge); + knowledge, + new DefaultKeyValueSnapshotReaderManager( + new DelegatingTransactionKeyValueServiceManager(keyValueService), + transactionService, + false, + new DefaultOrphanedSentinelDeleter(sweepStrategyManager::get, typedDeleteExecutor), + typedDeleteExecutor)); Supplier conditionSupplier = mock(Supplier.class); when(conditionSupplier.get()).thenReturn(ALWAYS_FAILS_CONDITION).thenReturn(PreCommitConditions.NO_OP); @@ -1766,7 +1826,7 @@ public void keepNumberOfExpectedCellsTheSameIfNoneCached() { PathTypeTracker pathTypeTracker = PathTypeTrackers.constructSynchronousTracker(); SnapshotTransaction spiedSnapshotTransaction = - spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager, pathTypeTracker)); + spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager)); Transaction transaction = transactionWrapper.apply(spiedSnapshotTransaction, pathTypeTracker); // Fetching 3 cells, but expect only 2 to be present, for example @@ -1785,9 +1845,7 @@ public void keepNumberOfExpectedCellsTheSameIfNoneCached() { eq("getWithExpectedNumberOfCells"), eq(TABLE_SWEPT_THOROUGH), eq(Set.of(TEST_CELL, TEST_CELL_2, TEST_CELL_3)), - eq(2L), - any(), - any()); + eq(2L)); } @Test @@ -1805,7 +1863,7 @@ public void reduceCachedCellsFromNumberOfExpectedCells() { PathTypeTracker pathTypeTracker = PathTypeTrackers.constructSynchronousTracker(); SnapshotTransaction spiedSnapshotTransaction = - spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager, pathTypeTracker)); + spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager)); Transaction transaction = transactionWrapper.apply(spiedSnapshotTransaction, pathTypeTracker); // Fetching 3 cells, but expect only 2 to be present, for example @@ -1824,9 +1882,7 @@ public void reduceCachedCellsFromNumberOfExpectedCells() { eq("getWithExpectedNumberOfCells"), eq(TABLE_SWEPT_THOROUGH), eq(Set.of(TEST_CELL_2, TEST_CELL_3)), // Don't expect to ask for cell1 because it's cached - eq(1L), - any(), - any()); + eq(1L)); } @Test @@ -1844,7 +1900,7 @@ public void keepNumberOfExpectedCellsIfCachedWithEmptyValue() { PathTypeTracker pathTypeTracker = PathTypeTrackers.constructSynchronousTracker(); SnapshotTransaction spiedSnapshotTransaction = - spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager, pathTypeTracker)); + spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager)); Transaction transaction = transactionWrapper.apply(spiedSnapshotTransaction, pathTypeTracker); // Fetching 3 cells, but expect only 2 to be present, for example @@ -1863,9 +1919,7 @@ public void keepNumberOfExpectedCellsIfCachedWithEmptyValue() { eq("getWithExpectedNumberOfCells"), eq(TABLE_SWEPT_THOROUGH), eq(Set.of(TEST_CELL_2, TEST_CELL_3)), // Don't expect to ask for cell1 because it's cached - eq(2L), - any(), - any()); + eq(2L)); } @Test @@ -1889,7 +1943,7 @@ public void dontFetchCellsIfAllCachedButPropagateWithEmptyRequest() { PathTypeTracker pathTypeTracker = PathTypeTrackers.constructSynchronousTracker(); SnapshotTransaction spiedSnapshotTransaction = - spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager, pathTypeTracker)); + spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager)); Transaction transaction = transactionWrapper.apply(spiedSnapshotTransaction, pathTypeTracker); Result, MoreCellsPresentThanExpectedException> result = @@ -1900,12 +1954,7 @@ public void dontFetchCellsIfAllCachedButPropagateWithEmptyRequest() { verify(spiedTimeLockService, never()).refreshLockLeases(any()); verify(spiedSnapshotTransaction) .getInternal( - eq("getWithExpectedNumberOfCells"), - eq(TABLE_SWEPT_THOROUGH), - eq(ImmutableSet.of()), - anyLong(), - any(), - any()); + eq("getWithExpectedNumberOfCells"), eq(TABLE_SWEPT_THOROUGH), eq(ImmutableSet.of()), anyLong()); } @Test @@ -1928,7 +1977,7 @@ public void throwsIfAllValuesCachedButMoreThanExpectedPresent() { PathTypeTracker pathTypeTracker = PathTypeTrackers.constructSynchronousTracker(); SnapshotTransaction spiedSnapshotTransaction = - spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager, pathTypeTracker)); + spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager)); Transaction transaction = transactionWrapper.apply(spiedSnapshotTransaction, pathTypeTracker); Result, MoreCellsPresentThanExpectedException> result = @@ -1944,8 +1993,7 @@ public void throwsIfAllValuesCachedButMoreThanExpectedPresent() { verify(spiedTimeLockService, never()).refreshLockLeases(any()); verify(spiedSnapshotTransaction, never()) - .getInternal( - eq("getWithExpectedNumberOfCells"), eq(TABLE_SWEPT_THOROUGH), any(), anyLong(), any(), any()); + .getInternal(eq("getWithExpectedNumberOfCells"), eq(TABLE_SWEPT_THOROUGH), any(), anyLong()); } @Test @@ -1964,7 +2012,7 @@ public void canReadWithExpectedSizeOneAfterDeletingPresentCellAndWritingToAnothe PathTypeTracker pathTypeTracker = PathTypeTrackers.constructSynchronousTracker(); SnapshotTransaction spiedSnapshotTransaction = - spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager, pathTypeTracker)); + spy(getSnapshotTransactionWith(transactionTs, res, mockLockWatchManager)); Transaction transaction = transactionWrapper.apply(spiedSnapshotTransaction, pathTypeTracker); // Deleting existing cell and writing to a new one to simulate change from LOCAL -> REMOTE or vice versa. @@ -1984,9 +2032,7 @@ public void canReadWithExpectedSizeOneAfterDeletingPresentCellAndWritingToAnothe eq("getWithExpectedNumberOfCells"), eq(TABLE_SWEPT_THOROUGH), eq(Set.of(TEST_CELL, TEST_CELL_2)), - eq(1L), - any(), - any()); + eq(1L)); } private TransactionScopedCache createCacheWithEntry(TableReference table, Cell cell, byte[] value) { @@ -3290,11 +3336,9 @@ private Transaction getSnapshotTransactionWith( } private SnapshotTransaction getSnapshotTransactionWith( - long transactionTs, - LockImmutableTimestampResponse res, - LockWatchManagerInternal mockLockWatchManager, - PathTypeTracker pathTypeTracker) { + long transactionTs, LockImmutableTimestampResponse res, LockWatchManagerInternal mockLockWatchManager) { LongSupplier startTimestampSupplier = Suppliers.ofInstance(transactionTs)::get; + return new SnapshotTransaction( metricsManager, txnKeyValueServiceManager.getTransactionKeyValueService(startTimestampSupplier), @@ -3308,7 +3352,6 @@ private SnapshotTransaction getSnapshotTransactionWith( SweepStrategyManagers.createDefault(keyValueService), res.getImmutableTimestamp(), Optional.of(res.getLock()), - PreCommitConditions.NO_OP, AtlasDbConstraintCheckingMode.NO_CONSTRAINT_CHECKING, null, TransactionReadSentinelBehavior.THROW_EXCEPTION, @@ -3317,17 +3360,33 @@ private SnapshotTransaction getSnapshotTransactionWith( getRangesExecutor, defaultGetRangesConcurrency, MultiTableSweepQueueWriter.NO_OP, - new DefaultDeleteExecutor( - txnKeyValueServiceManager.getKeyValueService().orElseThrow(), - MoreExecutors.newDirectExecutorService()), + typedDeleteExecutor, true, transactionConfig::get, ConflictTracer.NO_OP, tableLevelMetricsController, knowledge, - // For tests this is fine - this keeps a reference to res, which is strictly speaking inefficient. + keyValueSnapshotReaderManager, createCommitTimestampLoader( - transactionTs, res::getImmutableTimestamp, Optional.of(res.getLock()), timelockService)); + transactionTs, res::getImmutableTimestamp, Optional.of(res.getLock()), timelockService), + createPreCommitConditionValidator( + Optional.of(res.getLock()), PreCommitConditions.NO_OP, timelockService, true)); + } + + private DefaultPreCommitRequirementValidator createPreCommitConditionValidator( + Optional immutableTsLock, + PreCommitCondition condition, + TimelockService timelockService, + boolean validateLocksOnReads) { + return new DefaultPreCommitRequirementValidator( + condition, + sweepStrategyManager, + transactionConfig::get, + new DefaultLockRefresher(timelockService), + immutableTsLock, + validateLocksOnReads, + TransactionOutcomeMetrics.create( + TransactionMetrics.of(metricsManager.getTaggedRegistry()), metricsManager.getTaggedRegistry())); } private Transaction getSnapshotTransactionWith( @@ -3353,6 +3412,9 @@ private Transaction getSnapshotTransactionWith( boolean validateLocksOnReads, Map tableConflictHandlers) { PathTypeTracker pathTypeTracker = PathTypeTrackers.constructSynchronousTracker(); + + Optional immutableTimestampLock = Optional.of(lockImmutableTimestampResponse.getLock()); + SnapshotTransaction transaction = new SnapshotTransaction( metricsManager, txnKeyValueServiceManager.getTransactionKeyValueService(startTs::get), @@ -3365,7 +3427,6 @@ private Transaction getSnapshotTransactionWith( SweepStrategyManagers.createDefault(keyValueService), lockImmutableTimestampResponse.getImmutableTimestamp(), Optional.of(lockImmutableTimestampResponse.getLock()), - preCommitCondition, AtlasDbConstraintCheckingMode.NO_CONSTRAINT_CHECKING, null, TransactionReadSentinelBehavior.THROW_EXCEPTION, @@ -3374,19 +3435,20 @@ private Transaction getSnapshotTransactionWith( getRangesExecutor, defaultGetRangesConcurrency, MultiTableSweepQueueWriter.NO_OP, - new DefaultDeleteExecutor( - txnKeyValueServiceManager.getKeyValueService().orElseThrow(), - MoreExecutors.newDirectExecutorService()), + typedDeleteExecutor, validateLocksOnReads, transactionConfig::get, ConflictTracer.NO_OP, tableLevelMetricsController, knowledge, + keyValueSnapshotReaderManager, createCommitTimestampLoader( startTs.get(), lockImmutableTimestampResponse::getImmutableTimestamp, Optional.of(lockImmutableTimestampResponse.getLock()), - timelockService)); + timelockService), + createPreCommitConditionValidator( + immutableTimestampLock, preCommitCondition, timelockService, validateLocksOnReads)); return transactionWrapper.apply(transaction, pathTypeTracker); } diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/CommitLockTest.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/CommitLockTest.java index 3229540c5e6..f6c52dcb2f5 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/CommitLockTest.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/CommitLockTest.java @@ -179,7 +179,8 @@ private TransactionManager createTransactionManager(ConflictHandler conflictHand timestampCache, MultiTableSweepQueueWriter.NO_OP, knowledge, - MoreExecutors.newDirectExecutorService()); + MoreExecutors.newDirectExecutorService(), + keyValueSnapshotReaderManager); transactionManager.overrideConflictHandlerForTable(TEST_TABLE, conflictHandler); return transactionManager; } diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SerializableTransactionManagerTest.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SerializableTransactionManagerTest.java index 915daf0770e..a987bb757f9 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SerializableTransactionManagerTest.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SerializableTransactionManagerTest.java @@ -37,6 +37,7 @@ import com.palantir.atlasdb.keyvalue.impl.DelegatingTransactionKeyValueServiceManager; import com.palantir.atlasdb.sweep.queue.MultiTableSweepQueueWriter; import com.palantir.atlasdb.transaction.ImmutableTransactionConfig; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; import com.palantir.atlasdb.transaction.api.KeyValueServiceStatus; import com.palantir.atlasdb.transaction.api.TransactionManager; import com.palantir.atlasdb.transaction.impl.metrics.DefaultMetricsFilterEvaluationContext; @@ -274,6 +275,7 @@ public void callbackCanCallTmMethodsEvenThoughTmStillThrows() { private TransactionManager getManagerWithCallback( boolean initializeAsync, Callback callBack, ScheduledExecutorService executor) { + DeleteExecutor defaultDeleteExecutor = DefaultDeleteExecutor.createDefault(mockKvs); return SerializableTransactionManager.create( metricsManager, new DelegatingTransactionKeyValueServiceManager(mockKvs), @@ -300,7 +302,14 @@ private TransactionManager getManagerWithCallback( ConflictTracer.NO_OP, DefaultMetricsFilterEvaluationContext.createDefault(), Optional.empty(), - knowledge); + knowledge, + defaultDeleteExecutor, + new DefaultKeyValueSnapshotReaderManager( + new DelegatingTransactionKeyValueServiceManager(mockKvs), + mock(TransactionService.class), + false, + mock(DefaultOrphanedSentinelDeleter.class), + defaultDeleteExecutor)); } private void nothingInitialized() { diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionManagerTest.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionManagerTest.java index 726a12ac763..8ff14f2337a 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionManagerTest.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionManagerTest.java @@ -40,6 +40,7 @@ import com.palantir.atlasdb.sweep.queue.MultiTableSweepQueueWriter; import com.palantir.atlasdb.transaction.ImmutableTransactionConfig; import com.palantir.atlasdb.transaction.api.AtlasDbConstraintCheckingMode; +import com.palantir.atlasdb.transaction.api.DeleteExecutor; import com.palantir.atlasdb.transaction.api.OpenTransaction; import com.palantir.atlasdb.transaction.impl.metrics.DefaultMetricsFilterEvaluationContext; import com.palantir.atlasdb.transaction.knowledge.TransactionKnowledgeComponents; @@ -98,6 +99,7 @@ public class SnapshotTransactionManagerTest { @BeforeEach public void setUp() { timestampService = inMemoryTimelockClassExtension.getManagedTimestampService(); + SweepStrategyManager sweepStrategyManager = SweepStrategyManagers.createDefault(keyValueService); snapshotTransactionManager = new SnapshotTransactionManager( metricsManager, transactionKeyValueServiceManager, @@ -108,7 +110,7 @@ public void setUp() { mock(TransactionService.class), () -> AtlasDbConstraintCheckingMode.FULL_CONSTRAINT_CHECKING_THROWS_EXCEPTIONS, null, - null, + sweepStrategyManager, cleaner, false, TransactionTestConstants.GET_RANGES_THREAD_POOL_SIZE, @@ -121,7 +123,13 @@ public void setUp() { ConflictTracer.NO_OP, DefaultMetricsFilterEvaluationContext.createDefault(), Optional.empty(), - knowledge); + knowledge, + new DefaultKeyValueSnapshotReaderManager( + transactionKeyValueServiceManager, + mock(TransactionService.class), + false, + new DefaultOrphanedSentinelDeleter(sweepStrategyManager::get, deleteExecutor), + deleteExecutor)); } @Test @@ -178,7 +186,14 @@ public void canCloseTransactionManagerWithNonCloseableLockService() { ConflictTracer.NO_OP, DefaultMetricsFilterEvaluationContext.createDefault(), Optional.empty(), - knowledge); + knowledge, + new DefaultKeyValueSnapshotReaderManager( + transactionKeyValueServiceManager, + mock(TransactionService.class), + false, + new DefaultOrphanedSentinelDeleter( + SweepStrategyManagers.createDefault(keyValueService)::get, deleteExecutor), + deleteExecutor)); newTransactionManager.close(); // should not throw } @@ -309,7 +324,7 @@ private SnapshotTransactionManager createSnapshotTransactionManager( mock(TransactionService.class), () -> null, null, - null, + SweepStrategyManagers.createDefault(keyValueService), cleaner, false, TransactionTestConstants.GET_RANGES_THREAD_POOL_SIZE, @@ -324,6 +339,13 @@ private SnapshotTransactionManager createSnapshotTransactionManager( ConflictTracer.NO_OP, DefaultMetricsFilterEvaluationContext.createDefault(), Optional.empty(), - knowledge); + knowledge, + new DefaultKeyValueSnapshotReaderManager( + transactionKeyValueServiceManager, + mock(TransactionService.class), + false, + new DefaultOrphanedSentinelDeleter( + SweepStrategyManagers.createDefault(keyValueService)::get, deleteExecutor), + deleteExecutor)); } } diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/TransactionManagerTest.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/TransactionManagerTest.java index 76baca83662..3c44760a95d 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/TransactionManagerTest.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/TransactionManagerTest.java @@ -269,7 +269,8 @@ private TransactionManager setupTransactionManager() { ConflictTracer.NO_OP, DefaultMetricsFilterEvaluationContext.createDefault(), Optional.empty(), - knowledge); + knowledge, + keyValueSnapshotReaderManager); when(timelock.getFreshTimestamp()).thenReturn(1L); when(timelock.lockImmutableTimestamp())