From e3562830d1f66d4835bc1f0d0e039f9ad87eaaeb Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Tue, 7 Jul 2026 15:46:11 -0700 Subject: [PATCH 01/16] [Spark] Row-level concurrency for concurrent DML via deletion-vector merge POC for issue #7057. Adds a config-gated conflict-resolution phase to ConflictChecker that resolves 'same physical file' conflicts between concurrent DML. It decodes both transactions' deletion vectors and, when the newly-deleted rows are disjoint, merges them (dv_win UNION dv_cur), writes a new DV file, and rebases the losing transaction onto the winner's post-image. The delete/read, delete/delete, and append checks skip resolved paths; rewrite-only DML (DELETE/UPDATE) winners also skip the append check. DV-only (row tracking not required). Gated by spark.databricks.delta.rowLevelConcurrency.enabled (default off). Adds RowLevelConcurrencySuite. Co-Authored-By: Claude Opus 4.8 --- .../spark/sql/delta/ConflictChecker.scala | 234 +++++++++++++- .../sql/delta/sources/DeltaSQLConf.scala | 13 + .../sql/delta/RowLevelConcurrencySuite.scala | 297 ++++++++++++++++++ 3 files changed, 538 insertions(+), 6 deletions(-) create mode 100644 spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala index d5cd7be1beb..b77bca2f9f1 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala @@ -17,24 +17,28 @@ package org.apache.spark.sql.delta // scalastyle:off import.ordering.noEmptyLine +import java.util.UUID import java.util.concurrent.TimeUnit import scala.collection.mutable -import org.apache.spark.sql.delta.DeltaOperations.{OP_SET_TBLPROPERTIES, ROW_TRACKING_BACKFILL_OPERATION_NAME, ROW_TRACKING_UNBACKFILL_OPERATION_NAME} +import org.apache.spark.sql.delta.DeltaOperations.{OP_DELETE, OP_SET_TBLPROPERTIES, OP_UPDATE, ROW_TRACKING_BACKFILL_OPERATION_NAME, ROW_TRACKING_UNBACKFILL_OPERATION_NAME} import org.apache.spark.sql.delta.RowId.RowTrackingMetadataDomain import org.apache.spark.sql.delta.actions._ import org.apache.spark.sql.delta.catalog.DeltaTableV2 +import org.apache.spark.sql.delta.commands.DeletionVectorUtils +import org.apache.spark.sql.delta.deletionvectors.{RoaringBitmapArray, RoaringBitmapArrayFormat} import org.apache.spark.sql.delta.logging.DeltaLogKeys import org.apache.spark.sql.delta.metering.DeltaLogging import org.apache.spark.sql.delta.sources.DeltaSourceUtils import org.apache.spark.sql.delta.sources.DeltaSQLConf +import org.apache.spark.sql.delta.storage.dv.DeletionVectorStore import org.apache.spark.sql.delta.util.DeltaSparkPlanUtils.CheckDeterministicOptions import org.apache.spark.sql.delta.util.FileNames import io.delta.storage.commit.UpdatedActions import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient import io.delta.storage.commit.uniform.UniformMetadata -import org.apache.hadoop.fs.FileStatus +import org.apache.hadoop.fs.{FileStatus, Path} import org.apache.spark.internal.{MDC, MessageWithContext} import org.apache.spark.sql.{DataFrame, SparkSession} @@ -229,6 +233,13 @@ private[delta] class ConflictChecker( protected var currentTransactionInfo: CurrentTransactionInfo = initialCurrentTransactionInfo + /** + * Paths of files whose "same physical file" conflict with the winning transaction was resolved at + * the row level by [[resolveRowLevelConflicts]] (deletion vectors merged). The file-level delete + * and append checks skip these paths, since they have already been reconciled. + */ + private val rowLevelResolvedPaths = mutable.Set.empty[String] + protected def recordSkippedPhase(phase: String): Unit = timingStats += phase -> 0 /** @@ -300,6 +311,12 @@ private[delta] class ConflictChecker( // Update the table version in newly added type widening metadata. updateTypeWideningMetadata() + // Row-level concurrency: try to resolve "same physical file" conflicts by merging deletion + // vectors before the file-level checks run, so that concurrent DML touching disjoint rows of + // the same file no longer aborts. Runs after row-ID reassignment so merged files keep stable + // base row IDs. + resolveRowLevelConflicts() + // Data file checks. checkForAddedFilesThatShouldHaveBeenReadByCurrentTxn() checkForDeletedFilesAgainstCurrentTxnReadFiles() @@ -1117,6 +1134,204 @@ private[delta] class ConflictChecker( false } + /** Whether row-level concurrency resolution is enabled and applicable to this table. */ + private lazy val rowLevelConcurrencyEnabled: Boolean = + spark.conf.get(DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_ENABLED) && + DeletionVectorUtils.deletionVectorsWritable( + currentTransactionInfo.protocol, currentTransactionInfo.metadata) + + /** The operation name of the winning commit, if available. */ + private lazy val winningOperationName: Option[String] = + winningCommitSummary.commitInfo.map(_.operation) + + /** + * Whether the winning commit is a row-level DML that only rewrites or removes existing rows + * (DELETE or UPDATE), i.e. it introduces no net-new logical rows. Files added by such a commit + * are rewrites of rows that the current transaction already observed in its read snapshot (or + * re-adds of files masked by a deletion vector), so they cannot introduce phantom rows for the + * current transaction. MERGE is intentionally excluded because it may insert net-new rows. + */ + private lazy val winningCommitIsRewriteOnlyRowLevelDml: Boolean = { + val usesDeletionVectors = winningCommitSummary.addedFiles.exists(_.deletionVector != null) + usesDeletionVectors && winningOperationName.exists { + case OP_DELETE | OP_UPDATE => true + case _ => false + } + } + + /** + * Whether a file added by the winning transaction can be skipped in the added-files (append) + * conflict check thanks to row-level concurrency resolution. This is true when: + * 1. the file's "same physical file" conflict was already reconciled by merging deletion + * vectors ([[rowLevelResolvedPaths]]), or + * 2. the winning commit is a rewrite-only row-level DML (DELETE/UPDATE) whose added files + * cannot introduce phantom rows (see [[winningCommitIsRewriteOnlyRowLevelDml]]). + */ + private def canSkipAddedFileForRowLevelConcurrency(addFile: AddFile): Boolean = { + if (!rowLevelConcurrencyEnabled) return false + rowLevelResolvedPaths.contains(addFile.path) || winningCommitIsRewriteOnlyRowLevelDml + } + + /** + * Resolves "same physical file" conflicts with the winning transaction at the row level. + * + * A DV-based DELETE/UPDATE emits, for each touched file `P`, a `RemoveFile(P)` (tombstone of the + * pre-image) and an `AddFile(P)` carrying a larger deletion vector. When both the winning and the + * current transaction touch the same file `P` this way, the two operations are logically + * independent as long as they mark *different* rows deleted. For every such shared file we: + * 1. decode the winning DV, the current DV and their common base DV (from the pre-image + * `RemoveFile`) as [[RoaringBitmapArray]]s; + * 2. check whether the newly-deleted rows are disjoint, i.e. `(dv_win INTERSECT dv_cur) MINUS + * dv_base` is empty. If they overlap, this is a genuine row-level conflict and we leave the + * file for the standard checks to abort; + * 3. on disjoint sets, merge the DVs (`dv_win UNION dv_cur`), persist a new DV file, + * and rebase the current transaction onto the winner's post-image: the current `AddFile(P)` + * now carries the merged DV and the current `RemoveFile(P)` now tombstones the winner's + * `AddFile(P)`. + * + * Resolved paths are recorded in [[rowLevelResolvedPaths]] and skipped by the file-level delete + * and append checks. Row identity is preserved for free: the merged file is the same physical + * file, so its base row ID is unchanged and [[reassignOverlappingRowIds]] (already run) leaves it + * alone. Deletion vectors index physical row positions within one immutable Parquet file, so the + * merge needs no row tracking. + */ + private def resolveRowLevelConflicts(): Unit = { + if (!rowLevelConcurrencyEnabled) return + + // Winning transaction's DV updates: path present in both an AddFile (with a DV) and a + // RemoveFile of the winning commit. + val winningRemovedPaths = winningCommitSummary.removedFiles.map(_.path).toSet + val winningDvUpdates: Map[String, AddFile] = winningCommitSummary.addedFiles.iterator + .filter(a => a.deletionVector != null && winningRemovedPaths.contains(a.path)) + .map(a => a.path -> a) + .toMap + if (winningDvUpdates.isEmpty) return + + // Current transaction's DV updates, indexed by path. + val currentAddByPath = currentTransactionInfo.actions.collect { + case a: AddFile if a.deletionVector != null => a.path -> a + }.toMap + val currentRemoveByPath = currentTransactionInfo.actions.collect { + case r: RemoveFile => r.path -> r + }.toMap + + val sharedPaths = winningDvUpdates.keySet + .intersect(currentAddByPath.keySet) + .intersect(currentRemoveByPath.keySet) + if (sharedPaths.isEmpty) return + + recordTime("resolved-row-level-conflicts") { + val dvStore = DeletionVectorStore.createInstance(deltaLog.newDeltaHadoopConf()) + val tablePath = deltaLog.dataPath + + // path -> (rebased AddFile, rebased RemoveFile) + val replacements = mutable.Map.empty[String, (AddFile, RemoveFile)] + for (path <- sharedPaths) { + val winningAdd = winningDvUpdates(path) + val currentAdd = currentAddByPath(path) + val currentRemove = currentRemoveByPath(path) + + def bitmapOf(dv: DeletionVectorDescriptor): RoaringBitmapArray = + readDeletionVectorOrEmpty(dvStore, dv, tablePath) + val baseBitmap = bitmapOf(currentRemove.deletionVector) + val winningBitmap = bitmapOf(winningAdd.deletionVector) + val currentBitmap = bitmapOf(currentAdd.deletionVector) + + // `baseBitmap` is the DV of `P` at the current txn's read time (carried on its RemoveFile); + // for a 2-way conflict it equals the winner's pre-image too. Both `dv_win` and `dv_cur` are + // supersets of it (a DV only grows), so the newly-deleted rows are `dv \ base` on each side + // and their overlap is `(dv_win INTERSECT dv_cur) MINUS base`. If empty, the two txns + // touched disjoint rows and the schedule `current ; winner` is a valid serialization under + // both WriteSerializable and Serializable (the winner's rewrites/deletes are of rows the + // current txn did not touch), so merging is safe. If non-empty, the same row was touched by + // both -> genuine conflict, left for the standard checks. (For 3+ way chains `base` becomes + // previous winner's DV rather than the original pre-image; the merge stays correct and the + // overlap test stays conservative.) + val newlyDeletedOverlap = winningBitmap.copy() + newlyDeletedOverlap.and(currentBitmap) + newlyDeletedOverlap.andNot(baseBitmap) + + if (newlyDeletedOverlap.isEmpty) { + // Disjoint: merge the deletion vectors and rebase onto the winner's post-image. + val mergedBitmap = winningBitmap.copy() + mergedBitmap.merge(currentBitmap) + val mergedDescriptor = writeMergedDeletionVector(dvStore, tablePath, mergedBitmap) + // Keep the current AddFile's identity (base row ID / default row commit version already + // reconciled by the row-ID phases) but point it at the merged DV. + val rebasedAdd = currentAdd + .copy(deletionVector = mergedDescriptor, dataChange = true) + .withoutTightBoundStats + // Tombstone the winner's now-live AddFile (carries the winning DV) instead of the stale + // pre-image. + val rebasedRemove = winningAdd.removeWithTimestamp() + replacements(path) = (rebasedAdd, rebasedRemove) + rowLevelResolvedPaths += path + } + // else: overlapping row-level modification -> genuine conflict, leave for standard checks. + } + + if (replacements.nonEmpty) { + val newActions = currentTransactionInfo.actions.map { + case a: AddFile if replacements.contains(a.path) => replacements(a.path)._1 + case r: RemoveFile if replacements.contains(r.path) => replacements(r.path)._2 + case other => other + } + // Resolved files are no longer "read" for the purposes of the delete-read check. + val newReadFiles = currentTransactionInfo.readFiles + .filterNot(f => rowLevelResolvedPaths.contains(f.path)) + currentTransactionInfo = + currentTransactionInfo.copy(actions = newActions, readFiles = newReadFiles) + + recordDeltaEvent( + deltaLog, + opType = "delta.rowLevelConcurrency.deletionVectorsMerged", + data = Map( + "winningCommitVersion" -> winningCommitVersion, + "resolvedPaths" -> rowLevelResolvedPaths.size, + "winningOperation" -> winningOperationName.getOrElse("UNKNOWN"))) + } + } + } + + /** Reads a deletion vector into a [[RoaringBitmapArray]], returning an empty bitmap for none. */ + private def readDeletionVectorOrEmpty( + dvStore: DeletionVectorStore, + dv: DeletionVectorDescriptor, + tablePath: Path): RoaringBitmapArray = { + if (dv == null || dv.isEmpty) new RoaringBitmapArray() else dvStore.read(dv, tablePath) + } + + /** + * Persists a merged bitmap to a new deletion vector file and returns its descriptor. + * + * NOTE: this writes a DV file as a side effect of conflict resolution. If the commit ultimately + * fails or is retried against another winning version, the file is orphaned and later reclaimed + * by VACUUM (same lifecycle as any DV written by DML). This mirrors how the DML write path + * persists DVs (see `DeletionVectorWriter.storeSerializedBitmap`). + */ + private def writeMergedDeletionVector( + dvStore: DeletionVectorStore, + tablePath: Path, + bitmap: RoaringBitmapArray): DeletionVectorDescriptor = { + // An empty DV has no on-disk representation (matches DeletionVectorWriter). + if (bitmap.isEmpty) return DeletionVectorDescriptor.EMPTY + val tablePathWithFs = dvStore.pathWithFileSystem(tablePath) + val fileId = UUID.randomUUID() + val writer = dvStore.createWriter(dvStore.generateFileNameInTable(tablePathWithFs, fileId)) + try { + val serialized = DeletionVectorUtils.serialize( + bitmap, RoaringBitmapArrayFormat.Portable, Some(tablePath)) + val range = writer.write(serialized) + DeletionVectorDescriptor.onDiskWithRelativePath( + id = fileId, + sizeInBytes = serialized.length, + cardinality = bitmap.cardinality, + offset = Some(range.offset)) + } finally { + writer.close() + } + } + /** * Check if the new files added by the already committed transactions should have been read by * the current transaction. @@ -1137,8 +1352,11 @@ private[delta] class ConflictChecker( Seq.empty } + val addedFilesAfterRowLevelResolution = + addedFilesToCheckForConflicts.filterNot(canSkipAddedFileForRowLevelConcurrency) + val fileMatchingPartitionReadPredicates = - getFirstFileMatchingPartitionPredicates(addedFilesToCheckForConflicts) + getFirstFileMatchingPartitionPredicates(addedFilesAfterRowLevelResolution) if (fileMatchingPartitionReadPredicates.nonEmpty) { throw DeltaErrors.concurrentAppendException( @@ -1160,7 +1378,7 @@ private[delta] class ConflictChecker( val readFilePaths = currentTransactionInfo.readFiles.map( f => f.path -> f.partitionValues).toMap val deleteReadOverlap = winningCommitSummary.removedFiles - .find(r => readFilePaths.contains(r.path)) + .find(r => readFilePaths.contains(r.path) && !rowLevelResolvedPaths.contains(r.path)) if (deleteReadOverlap.nonEmpty) { val partitionOpt = getPrettyPartitionMessage(readFilePaths(deleteReadOverlap.get.path)) throw DeltaErrors.concurrentDeleteReadException( @@ -1169,7 +1387,11 @@ private[delta] class ConflictChecker( winningCommitVersion, partitionOpt) } - if (winningCommitSummary.removedFiles.nonEmpty && currentTransactionInfo.readWholeTable) { + // Row-level concurrency: a removed file that was reconciled at the row level must not + // re-trigger the whole-table conflict either. + val unresolvedRemovedFiles = + winningCommitSummary.removedFiles.exists(r => !rowLevelResolvedPaths.contains(r.path)) + if (unresolvedRemovedFiles && currentTransactionInfo.readWholeTable) { throw DeltaErrors.concurrentDeleteReadException( winningCommitSummary.commitInfo, getTableNameOrPath, @@ -1190,7 +1412,7 @@ private[delta] class ConflictChecker( .collect { case r: RemoveFile => r.path -> r.partitionValues } .toMap val deleteOverlap = winningCommitSummary.removedFiles - .find(r => deletedFilePaths.contains(r.path)) + .find(r => deletedFilePaths.contains(r.path) && !rowLevelResolvedPaths.contains(r.path)) if (deleteOverlap.nonEmpty) { val partitionOpt = getPrettyPartitionMessage(deletedFilePaths(deleteOverlap.get.path)) throw DeltaErrors.concurrentDeleteDeleteException( diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala b/spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala index d4d9c813b6c..1277a464b86 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala @@ -534,6 +534,19 @@ trait DeltaSQLConfBase extends DeltaSQLConfUtils { .booleanConf .createWithDefault(true) + val DELTA_ROW_LEVEL_CONCURRENCY_ENABLED = + buildConf("rowLevelConcurrency.enabled") + .internal() + .doc( + """When enabled, the conflict checker attempts to resolve conflicts between concurrent + |DML operations at the row level using deletion vectors, instead of failing the + |transaction. For every "same physical file" conflict, it decodes both transactions' + |deletion vectors, checks whether the newly-deleted rows are disjoint, and on disjoint + |sets merges the deletion vectors and rebases the losing transaction onto the winner's + |post-image. Only active when deletion vectors are writable.""".stripMargin) + .booleanConf + .createWithDefault(false) + val DELTA_PROTOCOL_DEFAULT_WRITER_VERSION = buildConf("properties.defaults.minWriterVersion") .doc("The default writer protocol version to create new tables with, unless a feature " + diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala new file mode 100644 index 00000000000..f5377fc38f3 --- /dev/null +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -0,0 +1,297 @@ +/* + * Copyright (2021) The Delta Lake Project Authors. + * + * 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 org.apache.spark.sql.delta + +import java.io.File + +import scala.concurrent.duration.Duration + +import org.apache.spark.sql.delta.concurrency.PhaseLockingTestMixin +import org.apache.spark.sql.delta.concurrency.TransactionExecutionTestMixin +import org.apache.spark.sql.delta.sources.DeltaSQLConf +import org.apache.spark.sql.delta.test.DeltaSQLCommandTest + +import org.apache.spark.{SparkConf, SparkException} +import org.apache.spark.sql.{QueryTest, Row} +import org.apache.spark.sql.functions.lit +import org.apache.spark.sql.test.SharedSparkSession +import org.apache.spark.util.ThreadUtils + +/** + * End-to-end tests for deletion-vector-based row-level concurrency + * ([[DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_ENABLED]]). + * + * Two concurrent DML operations that touch the same physical file but modify disjoint rows should + * commit cleanly by merging their deletion vectors, instead of aborting the losing transaction. + */ +class RowLevelConcurrencySuite extends QueryTest + with SharedSparkSession + with DeltaSQLCommandTest + with PhaseLockingTestMixin + with TransactionExecutionTestMixin { + + // Enable deletion vectors on every table created by this suite. + override protected def sparkConf: SparkConf = super.sparkConf + .set(DeltaConfigs.ENABLE_DELETION_VECTORS_CREATION.defaultTablePropertyKey, "true") + + private def tableRef(dir: File): String = s"delta.`${dir.getCanonicalPath}`" + + /** Creates a single-file Delta table with `id` in [0, numRows) and deletion vectors enabled. */ + private def createSingleFileTableWithDVs(dir: File, numRows: Int = 100): DeltaLog = { + spark.range(start = 0, end = numRows, step = 1, numPartitions = 1) + .write.format("delta").mode("append").save(dir.getAbsolutePath) + val log = DeltaLog.forTable(spark, dir.getCanonicalPath) + val snapshot = log.update() + assert( + snapshot.metadata.configuration + .get(DeltaConfigs.ENABLE_DELETION_VECTORS_CREATION.key).contains("true"), + "deletion vectors must be enabled on the test table") + assert(snapshot.allFiles.collect().length === 1, "test table must have a single data file") + log + } + + /** A DELETE/UPDATE transaction that runs under the given row-level-concurrency setting. */ + private def sqlTxn(sqlText: String, rowLevelConcurrency: Boolean): () => Array[Row] = + () => { + withSQLConf( + DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_ENABLED.key -> rowLevelConcurrency.toString) { + sql(sqlText).collect() + } + Array.empty[Row] + } + + private def deletionVectorCardinalities(log: DeltaLog): Seq[Long] = + log.update().allFiles.collect() + .filter(_.deletionVector != null) + .map(_.deletionVector.cardinality) + .toSeq + + private def assertConcurrentModificationException(e: SparkException): Unit = { + val causeName = e.getCause.getClass.getName + assert( + Seq("ConcurrentAppend", "ConcurrentDeleteRead", "ConcurrentDeleteDelete") + .exists(causeName.contains), + s"Expected a concurrency conflict, got: $causeName") + } + + private def ids(dir: File): Seq[Long] = + spark.read.format("delta").load(dir.getAbsolutePath).select("id") + .collect().map(_.getLong(0)).sorted.toSeq + + // --------------------------------------------------------------------------- + // DELETE vs DELETE (same file) + // --------------------------------------------------------------------------- + + test("disjoint concurrent DELETEs on the same file both commit by merging deletion vectors") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => id == 10 || id == 20)) + // One surviving file carrying the merged deletion vector (cardinality 2). + assert(deletionVectorCardinalities(log) === Seq(2L)) + } + } + + test("overlapping concurrent DELETEs still conflict") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + // Clean abort: only the winner's delete (id=10) is applied; its DV has cardinality 1. + assert(ids(dir) === (0L to 99L).filterNot(_ == 10)) + assert(deletionVectorCardinalities(log) === Seq(1L)) + } + } + + test("feature disabled: disjoint concurrent DELETEs still conflict") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = false) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = false) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + // Only the winner's delete (id=20) is applied; DVs are still used, so its DV has cardinality 1. + assert(ids(dir) === (0L to 99L).filterNot(_ == 20)) + assert(deletionVectorCardinalities(log) === Seq(1L)) + } + } + + test("deletion vectors disabled: row-level concurrency gate is off, disjoint DELETEs conflict") { + withTempDir { dir => + createSingleFileTableWithDVs(dir) + sql(s"ALTER TABLE ${tableRef(dir)} SET TBLPROPERTIES " + + s"('${DeltaConfigs.ENABLE_DELETION_VECTORS_CREATION.key}' = 'false')") + val log = DeltaLog.forTable(spark, dir.getCanonicalPath) + // Flag ON, but DVs are not writable -> resolveRowLevelConflicts no-ops. + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + // Winner's delete (id=20) applied by rewriting the file (no DVs), so no DV is present. + assert(ids(dir) === (0L to 99L).filterNot(_ == 20)) + assert(deletionVectorCardinalities(log) === Seq.empty[Long]) + } + } + + // --------------------------------------------------------------------------- + // Layer 2: UPDATE (rewrite-only DML adds new data files) + // --------------------------------------------------------------------------- + + test("disjoint DELETE (loser) vs UPDATE (winner) both commit") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + // A (loser) deletes id=10; B (winner) updates id=20 -> 1020 (masks row 20, appends image). + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"UPDATE ${tableRef(dir)} SET id = 1020 WHERE id = 20", + rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === ((0L to 99L).filterNot(id => id == 10 || id == 20) :+ 1020L).sorted) + // The original file's merged DV masks rows 10 and 20; the updated image lives in a new file. + assert(deletionVectorCardinalities(log) === Seq(2L)) + } + } + + test("disjoint UPDATE vs UPDATE both commit") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + val txnA = sqlTxn(s"UPDATE ${tableRef(dir)} SET id = 1010 WHERE id = 10", + rowLevelConcurrency = true) + val txnB = sqlTxn(s"UPDATE ${tableRef(dir)} SET id = 1020 WHERE id = 20", + rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === + ((0L to 99L).filterNot(id => id == 10 || id == 20) ++ Seq(1010L, 1020L)).sorted) + // Original file's merged DV masks rows 10 and 20; updated images live in two new files. + assert(deletionVectorCardinalities(log) === Seq(2L)) + } + } + + test("overlapping UPDATE vs UPDATE (same row) still conflict") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + val txnA = sqlTxn(s"UPDATE ${tableRef(dir)} SET id = 1010 WHERE id = 20", + rowLevelConcurrency = true) + val txnB = sqlTxn(s"UPDATE ${tableRef(dir)} SET id = 2020 WHERE id = 20", + rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + // Only the winner's update (20 -> 2020) is applied; original file's DV masks row 20. + assert(ids(dir) === ((0L to 99L).filterNot(_ == 20) :+ 2020L).sorted) + assert(deletionVectorCardinalities(log) === Seq(1L)) + } + } + + // --------------------------------------------------------------------------- + // Winner fully removes the file -> not reconcilable -> conflict + // --------------------------------------------------------------------------- + + test("winner that fully removes a file conflicts with a concurrent row-level delete") { + withTempDir { dir => + // Two files: [0,50) and [50,100). + spark.range(start = 0, end = 100, step = 1, numPartitions = 2) + .write.format("delta").mode("append").save(dir.getAbsolutePath) + val log = DeltaLog.forTable(spark, dir.getCanonicalPath) + // A (loser) DV-deletes one row in the first file; B (winner) deletes the whole first file. + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id < 50", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + // Winner fully removed the first file (no DV); loser aborted cleanly. + assert(ids(dir) === (50L to 99L)) + assert(deletionVectorCardinalities(log) === Seq.empty[Long]) + } + } + + // --------------------------------------------------------------------------- + // N-way: three concurrent transactions on the same file + // --------------------------------------------------------------------------- + + test("three concurrent disjoint DELETEs on the same file all commit") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + val txnC = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 30", rowLevelConcurrency = true) + + // A starts; B commits; C commits; A commits last (reconciles against both B and C). + val (futureA, futureB, futureC) = + runTxnsWithOrder__A_Start__B__C__A_End(txnA, txnB, txnC) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + ThreadUtils.awaitResult(futureC, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => Set(10L, 20L, 30L).contains(id))) + assert(deletionVectorCardinalities(log) === Seq(3L)) + } + } + + // --------------------------------------------------------------------------- + // Partitioned table (DV merge is partition-agnostic) + // --------------------------------------------------------------------------- + + test("disjoint concurrent DELETEs on a partitioned table's file both commit") { + withTempDir { dir => + // Single partition p=0 with a single data file. + spark.range(start = 0, end = 100, step = 1, numPartitions = 1) + .withColumn("p", lit(0)) + .write.partitionBy("p").format("delta").mode("append").save(dir.getAbsolutePath) + val log = DeltaLog.forTable(spark, dir.getCanonicalPath) + assert(log.update().allFiles.collect().length === 1) + + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => id == 10 || id == 20)) + assert(deletionVectorCardinalities(log) === Seq(2L)) + } + } +} From b64e025e48986f05b7ed4b074e5625e70d404026 Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Thu, 30 Jul 2026 15:25:56 -0700 Subject: [PATCH 02/16] [Spark] Narrow row-level concurrency to sound same-file DV union Remove the unsound "Layer 2" append-suppression for rewrite-only DML winners. Keying the append-check skip on the winner's op type (DELETE/UPDATE) is unsound: an UPDATE can move a row into the loser's predicate (winner `SET x=15`, loser `DELETE WHERE x>10`, row was x=5), a genuine write-skew that suppressing the winner's image file would hide. `canSkipAddedFileForRowLevelConcurrency` now skips only the same-file DV union paths (`rowLevelResolvedPaths`). A rewrite-only DML winner's new image files flow into the standard added-files check (conservative abort, one-way safe); reconciling the provably-disjoint case via conflict-time data skipping is owned by Case 1 (#4/#8). Tests reframed to the Layer-1 envelope (9 tests, all passing): the two "both commit" UPDATE cases move to Case 1; DELETE-vs-UPDATE now asserts the image file conservatively conflicts. Co-Authored-By: Claude Opus 4.8 --- .../spark/sql/delta/ConflictChecker.scala | 38 +++++++-------- .../sql/delta/RowLevelConcurrencySuite.scala | 48 ++++++++----------- 2 files changed, 37 insertions(+), 49 deletions(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala index b77bca2f9f1..cb54f3449bb 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala @@ -22,7 +22,7 @@ import java.util.concurrent.TimeUnit import scala.collection.mutable -import org.apache.spark.sql.delta.DeltaOperations.{OP_DELETE, OP_SET_TBLPROPERTIES, OP_UPDATE, ROW_TRACKING_BACKFILL_OPERATION_NAME, ROW_TRACKING_UNBACKFILL_OPERATION_NAME} +import org.apache.spark.sql.delta.DeltaOperations.{OP_SET_TBLPROPERTIES, ROW_TRACKING_BACKFILL_OPERATION_NAME, ROW_TRACKING_UNBACKFILL_OPERATION_NAME} import org.apache.spark.sql.delta.RowId.RowTrackingMetadataDomain import org.apache.spark.sql.delta.actions._ import org.apache.spark.sql.delta.catalog.DeltaTableV2 @@ -1144,32 +1144,26 @@ private[delta] class ConflictChecker( private lazy val winningOperationName: Option[String] = winningCommitSummary.commitInfo.map(_.operation) - /** - * Whether the winning commit is a row-level DML that only rewrites or removes existing rows - * (DELETE or UPDATE), i.e. it introduces no net-new logical rows. Files added by such a commit - * are rewrites of rows that the current transaction already observed in its read snapshot (or - * re-adds of files masked by a deletion vector), so they cannot introduce phantom rows for the - * current transaction. MERGE is intentionally excluded because it may insert net-new rows. - */ - private lazy val winningCommitIsRewriteOnlyRowLevelDml: Boolean = { - val usesDeletionVectors = winningCommitSummary.addedFiles.exists(_.deletionVector != null) - usesDeletionVectors && winningOperationName.exists { - case OP_DELETE | OP_UPDATE => true - case _ => false - } - } - /** * Whether a file added by the winning transaction can be skipped in the added-files (append) - * conflict check thanks to row-level concurrency resolution. This is true when: - * 1. the file's "same physical file" conflict was already reconciled by merging deletion - * vectors ([[rowLevelResolvedPaths]]), or - * 2. the winning commit is a rewrite-only row-level DML (DELETE/UPDATE) whose added files - * cannot introduce phantom rows (see [[winningCommitIsRewriteOnlyRowLevelDml]]). + * conflict check thanks to row-level concurrency resolution. + * + * This is true only when the file's "same physical file" conflict was already reconciled by + * merging deletion vectors ([[resolveRowLevelConflicts]] recorded the path in + * [[rowLevelResolvedPaths]]). In that case the winner's re-added `AddFile(P)` carries the winning + * DV that we already folded into the current transaction's merged DV, so re-checking it would be + * a false conflict. + * + * We deliberately do NOT skip a rewrite-only DML winner's *new image* files here (an UPDATE + * writes updated row values to a fresh path). Those are ordinary non-blind changed-data files + * and can legitimately conflict: e.g. an UPDATE can move a row *into* the loser's predicate + * (winner `SET x = 15`, loser `DELETE WHERE x > 10`, row was `x = 5`), a genuine write-skew that + * the DV union cannot detect. They are arbitrated by the standard added-files check (and, when + * enabled, by conflict-time data skipping over their stats). */ private def canSkipAddedFileForRowLevelConcurrency(addFile: AddFile): Boolean = { if (!rowLevelConcurrencyEnabled) return false - rowLevelResolvedPaths.contains(addFile.path) || winningCommitIsRewriteOnlyRowLevelDml + rowLevelResolvedPaths.contains(addFile.path) } /** diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala index f5377fc38f3..c93b24ea870 100644 --- a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -37,6 +37,12 @@ import org.apache.spark.util.ThreadUtils * * Two concurrent DML operations that touch the same physical file but modify disjoint rows should * commit cleanly by merging their deletion vectors, instead of aborting the losing transaction. + * + * Scope note: this is the sound *same-file DV union*. A rewrite-only DML that also writes *new + * image* files (an UPDATE emits updated row values to a fresh path) is NOT reconciled here. Those + * image files are ordinary non-blind changed-data files that can carry a genuine conflict (a + * value-flip write-skew), so they fall back to today's abort. Reconciling them on proven + * non-overlap is conflict-time data skipping, owned by Case 1 (fork issue #4). */ class RowLevelConcurrencySuite extends QueryTest with SharedSparkSession @@ -138,7 +144,7 @@ class RowLevelConcurrencySuite extends QueryTest ThreadUtils.awaitResult(futureB, Duration.Inf) val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } assertConcurrentModificationException(e) - // Only the winner's delete (id=20) is applied; DVs are still used, so its DV has cardinality 1. + // Only the winner's delete (id=20) is applied; DVs still used, so its DV cardinality is 1. assert(ids(dir) === (0L to 99L).filterNot(_ == 20)) assert(deletionVectorCardinalities(log) === Seq(1L)) } @@ -165,10 +171,12 @@ class RowLevelConcurrencySuite extends QueryTest } // --------------------------------------------------------------------------- - // Layer 2: UPDATE (rewrite-only DML adds new data files) + // UPDATE image files: rewrite-only DML writes new data files that Layer-1 (DV union) does not + // reconcile. They conservatively conflict here; conflict-time data skipping (Case 1 / #4) + // reconciles the provably-disjoint case. // --------------------------------------------------------------------------- - test("disjoint DELETE (loser) vs UPDATE (winner) both commit") { + test("DELETE (loser) vs UPDATE (winner): winner image file conservatively conflicts") { withTempDir { dir => val log = createSingleFileTableWithDVs(dir) // A (loser) deletes id=10; B (winner) updates id=20 -> 1020 (masks row 20, appends image). @@ -177,31 +185,17 @@ class RowLevelConcurrencySuite extends QueryTest rowLevelConcurrency = true) val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) - ThreadUtils.awaitResult(futureA, Duration.Inf) ThreadUtils.awaitResult(futureB, Duration.Inf) - - assert(ids(dir) === ((0L to 99L).filterNot(id => id == 10 || id == 20) :+ 1020L).sorted) - // The original file's merged DV masks rows 10 and 20; the updated image lives in a new file. - assert(deletionVectorCardinalities(log) === Seq(2L)) - } - } - - test("disjoint UPDATE vs UPDATE both commit") { - withTempDir { dir => - val log = createSingleFileTableWithDVs(dir) - val txnA = sqlTxn(s"UPDATE ${tableRef(dir)} SET id = 1010 WHERE id = 10", - rowLevelConcurrency = true) - val txnB = sqlTxn(s"UPDATE ${tableRef(dir)} SET id = 1020 WHERE id = 20", - rowLevelConcurrency = true) - - val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) - ThreadUtils.awaitResult(futureA, Duration.Inf) - ThreadUtils.awaitResult(futureB, Duration.Inf) - - assert(ids(dir) === - ((0L to 99L).filterNot(id => id == 10 || id == 20) ++ Seq(1010L, 1020L)).sorted) - // Original file's merged DV masks rows 10 and 20; updated images live in two new files. - assert(deletionVectorCardinalities(log) === Seq(2L)) + // The shared file's DVs are disjoint and would merge, but the winner's UPDATE also writes an + // *image* file (new path) that the append check cannot prove disjoint from the loser's read + // without conflict-time data skipping (Case 1 / #4). It conservatively conflicts, so the + // loser aborts. This is one-way safe: it never reconciles a genuine value-flip write-skew + // (e.g. winner `SET x = 15`, loser `DELETE WHERE x > 10`), which the DV union cannot detect. + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + // Loser aborted cleanly: only the winner's update is applied (row 20 masked, 1020 appended). + assert(ids(dir) === ((0L to 99L).filterNot(_ == 20) :+ 1020L).sorted) + assert(deletionVectorCardinalities(log) === Seq(1L)) } } From 55f07ab7e8ecfd6ab87422bb5ac4931d534694d3 Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Mon, 3 Aug 2026 10:59:42 -0700 Subject: [PATCH 03/16] [Spark] Harden row-level DV-merge concurrency tests + scrub internal refs Add coverage for the deletion-vector-merge row-level concurrency path: - non-empty base DV: DELETE id=5 first, then two disjoint concurrent DELETEs, exercising the `(dv_win INTERSECT dv_cur) MINUS base` subtraction that every prior test left empty. - 3-way where the last (overlapping) txn aborts while the two disjoint ones commit. - disjoint DELETEs reconcile under explicit Serializable isolation. - row tracking: surviving rows keep their stable `_metadata.row_id` across the same-file DV merge. - change data feed: each deleted row is emitted once, attributed to the version that deleted it (winner at N, reconciled txn at N+1). Also scrub fork-internal taxonomy from the suite comments (drop "Case 1 / fork issue #4" and "Layer-1" wording) in favor of self-contained descriptions ("conflict-time reader-side data skipping (a separate change)", "same-file DV union"). No behavior change. Co-Authored-By: Claude Opus 4.8 --- .../sql/delta/RowLevelConcurrencySuite.scala | 168 ++++++++++++++++-- 1 file changed, 158 insertions(+), 10 deletions(-) diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala index c93b24ea870..6ee50ce2519 100644 --- a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -42,7 +42,7 @@ import org.apache.spark.util.ThreadUtils * image* files (an UPDATE emits updated row values to a fresh path) is NOT reconciled here. Those * image files are ordinary non-blind changed-data files that can carry a genuine conflict (a * value-flip write-skew), so they fall back to today's abort. Reconciling them on proven - * non-overlap is conflict-time data skipping, owned by Case 1 (fork issue #4). + * non-overlap is conflict-time reader-side data skipping (a separate change). */ class RowLevelConcurrencySuite extends QueryTest with SharedSparkSession @@ -56,10 +56,17 @@ class RowLevelConcurrencySuite extends QueryTest private def tableRef(dir: File): String = s"delta.`${dir.getCanonicalPath}`" - /** Creates a single-file Delta table with `id` in [0, numRows) and deletion vectors enabled. */ - private def createSingleFileTableWithDVs(dir: File, numRows: Int = 100): DeltaLog = { + /** + * Creates a single-file Delta table with `id` in [0, numRows) and deletion vectors enabled. + * `extraProperties` are applied as `delta.*` table properties at creation (e.g. row tracking or + * change data feed). + */ + private def createSingleFileTableWithDVs( + dir: File, + numRows: Int = 100, + extraProperties: Map[String, String] = Map.empty): DeltaLog = { spark.range(start = 0, end = numRows, step = 1, numPartitions = 1) - .write.format("delta").mode("append").save(dir.getAbsolutePath) + .write.format("delta").options(extraProperties).mode("append").save(dir.getAbsolutePath) val log = DeltaLog.forTable(spark, dir.getCanonicalPath) val snapshot = log.update() assert( @@ -118,6 +125,48 @@ class RowLevelConcurrencySuite extends QueryTest } } + test("disjoint concurrent DELETEs reconcile on a file that already carries a base DV") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + // Establish a non-empty base DV: delete id=5 and commit (the file now carries a DV of + // cardinality 1). Both concurrent txns below read this DV as their common base, so the + // overlap test must subtract it (`(dv_win INTERSECT dv_cur) MINUS base`); without that + // subtraction the shared row 5 would look like a false conflict and abort the merge. + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 5") + assert(deletionVectorCardinalities(log) === Seq(1L)) + + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => Set(5L, 10L, 20L).contains(id))) + // Base row 5 plus the two disjoint new deletes -> merged deletion vector cardinality 3. + assert(deletionVectorCardinalities(log) === Seq(3L)) + } + } + + test("disjoint concurrent DELETEs reconcile under Serializable isolation") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + sql(s"ALTER TABLE ${tableRef(dir)} SET TBLPROPERTIES " + + s"('${DeltaConfigs.ISOLATION_LEVEL.key}' = 'Serializable')") + // The `current ; winner` schedule of two disjoint-row deletes is a valid serialization even + // under Serializable (neither read a row the other wrote), so the DV union still commits. + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => id == 10 || id == 20)) + assert(deletionVectorCardinalities(log) === Seq(2L)) + } + } + test("overlapping concurrent DELETEs still conflict") { withTempDir { dir => val log = createSingleFileTableWithDVs(dir) @@ -171,9 +220,9 @@ class RowLevelConcurrencySuite extends QueryTest } // --------------------------------------------------------------------------- - // UPDATE image files: rewrite-only DML writes new data files that Layer-1 (DV union) does not - // reconcile. They conservatively conflict here; conflict-time data skipping (Case 1 / #4) - // reconciles the provably-disjoint case. + // UPDATE image files: rewrite-only DML writes new data files that same-file DV union does not + // reconcile. They conservatively conflict here; conflict-time reader-side data skipping (a + // separate change) reconciles the provably-disjoint case. // --------------------------------------------------------------------------- test("DELETE (loser) vs UPDATE (winner): winner image file conservatively conflicts") { @@ -188,9 +237,10 @@ class RowLevelConcurrencySuite extends QueryTest ThreadUtils.awaitResult(futureB, Duration.Inf) // The shared file's DVs are disjoint and would merge, but the winner's UPDATE also writes an // *image* file (new path) that the append check cannot prove disjoint from the loser's read - // without conflict-time data skipping (Case 1 / #4). It conservatively conflicts, so the - // loser aborts. This is one-way safe: it never reconciles a genuine value-flip write-skew - // (e.g. winner `SET x = 15`, loser `DELETE WHERE x > 10`), which the DV union cannot detect. + // without conflict-time reader-side data skipping (a separate change). It conservatively + // conflicts, so the loser aborts. This is one-way safe: it never reconciles a genuine + // value-flip write-skew (e.g. winner `SET x = 15`, loser `DELETE WHERE x > 10`), which the + // DV union cannot detect. val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } assertConcurrentModificationException(e) // Loser aborted cleanly: only the winner's update is applied (row 20 masked, 1020 appended). @@ -264,6 +314,30 @@ class RowLevelConcurrencySuite extends QueryTest } } + test("three concurrent DELETEs: two disjoint commit, the overlapping last one aborts") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + // B (id=20) and C (id=10) touch disjoint rows and both commit. A also deletes id=10 and, + // committing last, reconciles cleanly against B (disjoint) but then discovers its overlap + // with C on row 10, so it aborts. + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + val txnC = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + + // A starts; B commits; C commits (reading B's state); A commits last. + val (futureA, futureB, futureC) = + runTxnsWithOrder__A_Start__B__C__A_End(txnA, txnB, txnC) + ThreadUtils.awaitResult(futureB, Duration.Inf) + ThreadUtils.awaitResult(futureC, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + + // Only B (id=20) and C (id=10) applied; A aborted cleanly on the id=10 overlap. + assert(ids(dir) === (0L to 99L).filterNot(id => id == 10 || id == 20)) + assert(deletionVectorCardinalities(log) === Seq(2L)) + } + } + // --------------------------------------------------------------------------- // Partitioned table (DV merge is partition-agnostic) // --------------------------------------------------------------------------- @@ -288,4 +362,78 @@ class RowLevelConcurrencySuite extends QueryTest assert(deletionVectorCardinalities(log) === Seq(2L)) } } + + // --------------------------------------------------------------------------- + // Row tracking: the merge keeps the same physical file, so stable row IDs are preserved + // --------------------------------------------------------------------------- + + test("row tracking: reconciled disjoint DELETEs preserve surviving rows' stable row IDs") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir, + extraProperties = Map(DeltaConfigs.ROW_TRACKING_ENABLED.key -> "true")) + // Snapshot each row's stable row id before the concurrent deletes. + val before = spark.read.format("delta").load(dir.getAbsolutePath) + .select("id", "_metadata.row_id") + .collect().map(r => r.getLong(0) -> r.getLong(1)).toMap + + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => id == 10 || id == 20)) + assert(deletionVectorCardinalities(log) === Seq(2L)) + + val after = spark.read.format("delta").load(dir.getAbsolutePath) + .select("id", "_metadata.row_id") + .collect().map(r => r.getLong(0) -> r.getLong(1)).toMap + // Merging DVs on the same physical file leaves base row IDs untouched, so every surviving + // row keeps the exact stable row id it had before the concurrent deletes. + assert(after === (before -- Seq(10L, 20L))) + } + } + + // --------------------------------------------------------------------------- + // Change Data Feed: each deleted row is emitted once, at the version that deleted it + // --------------------------------------------------------------------------- + + test("change data feed: reconciled disjoint DELETEs each emit one delete at their own version") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir, + extraProperties = Map(DeltaConfigs.CHANGE_DATA_FEED.key -> "true")) + // Table creation is version 0; the winner commits at version 1 and the reconciled current + // txn at version 2. + val firstDeleteVersion = log.update().version + 1 + + // A starts first but commits last, so B (id=20) is the winner at version 1 and A (id=10) is + // the reconciled current txn at version 2. + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => id == 10 || id == 20)) + assert(deletionVectorCardinalities(log) === Seq(2L)) + + val changes = spark.read.format("delta") + .option("readChangeFeed", "true") + .option("startingVersion", firstDeleteVersion) + .load(dir.getAbsolutePath) + .select("id", "_change_type", "_commit_version") + .where("_change_type = 'delete'") + .collect() + .map(r => (r.getLong(0), r.getString(1), r.getLong(2))) + .sortBy(_._1) + .toSeq + // Each deleted row appears exactly once, attributed to the version that deleted it: the + // winner's id=20 at version 1 and the reconciled txn's id=10 at version 2. + assert(changes === Seq( + (10L, "delete", firstDeleteVersion + 1), + (20L, "delete", firstDeleteVersion))) + } + } } From 538ef84c3fd8409f49a44799a33ab0aab83f221c Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Tue, 4 Aug 2026 11:25:00 -0700 Subject: [PATCH 04/16] Extract row-level concurrency resolution into a self-typed trait Move the RLC deletion-vector merge (rowLevelConcurrencyEnabled, winningOperationName, canSkipAddedFileForRowLevelConcurrency, resolveRowLevelConflicts, the DV read/write helpers, and rowLevelResolvedPaths) out of the 1862-line ConflictChecker into RowLevelConcurrencyResolution -- a `self: ConflictChecker =>` trait mixed into the checker. ConflictChecker keeps only the call sites; `spark` and `winningCommitSummary` are widened to `protected val`, and the DV helpers stay `protected` so the OPTIMIZE-vs-DML reconciliation (which mixes into the same checker) can reuse them. Also add a concrete worked example to the resolveRowLevelConflicts doc: winner DELETEs {5,10}, current DELETEs {20,21}, disjoint over base {} -> merge to {5,10,20,21}; had current deleted row 5, overlap {5} -> abort. Co-Authored-By: Claude Opus 4.8 --- .../spark/sql/delta/ConflictChecker.scala | 212 +-------------- .../delta/RowLevelConcurrencyResolution.scala | 252 ++++++++++++++++++ 2 files changed, 257 insertions(+), 207 deletions(-) create mode 100644 spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala index cb54f3449bb..cf7a0446bdc 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala @@ -17,7 +17,6 @@ package org.apache.spark.sql.delta // scalastyle:off import.ordering.noEmptyLine -import java.util.UUID import java.util.concurrent.TimeUnit import scala.collection.mutable @@ -26,19 +25,16 @@ import org.apache.spark.sql.delta.DeltaOperations.{OP_SET_TBLPROPERTIES, ROW_TRA import org.apache.spark.sql.delta.RowId.RowTrackingMetadataDomain import org.apache.spark.sql.delta.actions._ import org.apache.spark.sql.delta.catalog.DeltaTableV2 -import org.apache.spark.sql.delta.commands.DeletionVectorUtils -import org.apache.spark.sql.delta.deletionvectors.{RoaringBitmapArray, RoaringBitmapArrayFormat} import org.apache.spark.sql.delta.logging.DeltaLogKeys import org.apache.spark.sql.delta.metering.DeltaLogging import org.apache.spark.sql.delta.sources.DeltaSourceUtils import org.apache.spark.sql.delta.sources.DeltaSQLConf -import org.apache.spark.sql.delta.storage.dv.DeletionVectorStore import org.apache.spark.sql.delta.util.DeltaSparkPlanUtils.CheckDeterministicOptions import org.apache.spark.sql.delta.util.FileNames import io.delta.storage.commit.UpdatedActions import io.delta.storage.commit.uccommitcoordinator.UCCommitCoordinatorClient import io.delta.storage.commit.uniform.UniformMetadata -import org.apache.hadoop.fs.{FileStatus, Path} +import org.apache.hadoop.fs.FileStatus import org.apache.spark.internal.{MDC, MessageWithContext} import org.apache.spark.sql.{DataFrame, SparkSession} @@ -220,11 +216,12 @@ object WinningCommitSummary { } private[delta] class ConflictChecker( - spark: SparkSession, + protected val spark: SparkSession, initialCurrentTransactionInfo: CurrentTransactionInfo, - winningCommitSummary: WinningCommitSummary, + protected val winningCommitSummary: WinningCommitSummary, isolationLevel: IsolationLevel) - extends DeltaLogging with ConflictCheckerPredicateElimination { + extends DeltaLogging with ConflictCheckerPredicateElimination + with RowLevelConcurrencyResolution { protected val winningCommitVersion = winningCommitSummary.commitVersion protected val startTimeMs = System.currentTimeMillis() @@ -233,13 +230,6 @@ private[delta] class ConflictChecker( protected var currentTransactionInfo: CurrentTransactionInfo = initialCurrentTransactionInfo - /** - * Paths of files whose "same physical file" conflict with the winning transaction was resolved at - * the row level by [[resolveRowLevelConflicts]] (deletion vectors merged). The file-level delete - * and append checks skip these paths, since they have already been reconciled. - */ - private val rowLevelResolvedPaths = mutable.Set.empty[String] - protected def recordSkippedPhase(phase: String): Unit = timingStats += phase -> 0 /** @@ -1134,198 +1124,6 @@ private[delta] class ConflictChecker( false } - /** Whether row-level concurrency resolution is enabled and applicable to this table. */ - private lazy val rowLevelConcurrencyEnabled: Boolean = - spark.conf.get(DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_ENABLED) && - DeletionVectorUtils.deletionVectorsWritable( - currentTransactionInfo.protocol, currentTransactionInfo.metadata) - - /** The operation name of the winning commit, if available. */ - private lazy val winningOperationName: Option[String] = - winningCommitSummary.commitInfo.map(_.operation) - - /** - * Whether a file added by the winning transaction can be skipped in the added-files (append) - * conflict check thanks to row-level concurrency resolution. - * - * This is true only when the file's "same physical file" conflict was already reconciled by - * merging deletion vectors ([[resolveRowLevelConflicts]] recorded the path in - * [[rowLevelResolvedPaths]]). In that case the winner's re-added `AddFile(P)` carries the winning - * DV that we already folded into the current transaction's merged DV, so re-checking it would be - * a false conflict. - * - * We deliberately do NOT skip a rewrite-only DML winner's *new image* files here (an UPDATE - * writes updated row values to a fresh path). Those are ordinary non-blind changed-data files - * and can legitimately conflict: e.g. an UPDATE can move a row *into* the loser's predicate - * (winner `SET x = 15`, loser `DELETE WHERE x > 10`, row was `x = 5`), a genuine write-skew that - * the DV union cannot detect. They are arbitrated by the standard added-files check (and, when - * enabled, by conflict-time data skipping over their stats). - */ - private def canSkipAddedFileForRowLevelConcurrency(addFile: AddFile): Boolean = { - if (!rowLevelConcurrencyEnabled) return false - rowLevelResolvedPaths.contains(addFile.path) - } - - /** - * Resolves "same physical file" conflicts with the winning transaction at the row level. - * - * A DV-based DELETE/UPDATE emits, for each touched file `P`, a `RemoveFile(P)` (tombstone of the - * pre-image) and an `AddFile(P)` carrying a larger deletion vector. When both the winning and the - * current transaction touch the same file `P` this way, the two operations are logically - * independent as long as they mark *different* rows deleted. For every such shared file we: - * 1. decode the winning DV, the current DV and their common base DV (from the pre-image - * `RemoveFile`) as [[RoaringBitmapArray]]s; - * 2. check whether the newly-deleted rows are disjoint, i.e. `(dv_win INTERSECT dv_cur) MINUS - * dv_base` is empty. If they overlap, this is a genuine row-level conflict and we leave the - * file for the standard checks to abort; - * 3. on disjoint sets, merge the DVs (`dv_win UNION dv_cur`), persist a new DV file, - * and rebase the current transaction onto the winner's post-image: the current `AddFile(P)` - * now carries the merged DV and the current `RemoveFile(P)` now tombstones the winner's - * `AddFile(P)`. - * - * Resolved paths are recorded in [[rowLevelResolvedPaths]] and skipped by the file-level delete - * and append checks. Row identity is preserved for free: the merged file is the same physical - * file, so its base row ID is unchanged and [[reassignOverlappingRowIds]] (already run) leaves it - * alone. Deletion vectors index physical row positions within one immutable Parquet file, so the - * merge needs no row tracking. - */ - private def resolveRowLevelConflicts(): Unit = { - if (!rowLevelConcurrencyEnabled) return - - // Winning transaction's DV updates: path present in both an AddFile (with a DV) and a - // RemoveFile of the winning commit. - val winningRemovedPaths = winningCommitSummary.removedFiles.map(_.path).toSet - val winningDvUpdates: Map[String, AddFile] = winningCommitSummary.addedFiles.iterator - .filter(a => a.deletionVector != null && winningRemovedPaths.contains(a.path)) - .map(a => a.path -> a) - .toMap - if (winningDvUpdates.isEmpty) return - - // Current transaction's DV updates, indexed by path. - val currentAddByPath = currentTransactionInfo.actions.collect { - case a: AddFile if a.deletionVector != null => a.path -> a - }.toMap - val currentRemoveByPath = currentTransactionInfo.actions.collect { - case r: RemoveFile => r.path -> r - }.toMap - - val sharedPaths = winningDvUpdates.keySet - .intersect(currentAddByPath.keySet) - .intersect(currentRemoveByPath.keySet) - if (sharedPaths.isEmpty) return - - recordTime("resolved-row-level-conflicts") { - val dvStore = DeletionVectorStore.createInstance(deltaLog.newDeltaHadoopConf()) - val tablePath = deltaLog.dataPath - - // path -> (rebased AddFile, rebased RemoveFile) - val replacements = mutable.Map.empty[String, (AddFile, RemoveFile)] - for (path <- sharedPaths) { - val winningAdd = winningDvUpdates(path) - val currentAdd = currentAddByPath(path) - val currentRemove = currentRemoveByPath(path) - - def bitmapOf(dv: DeletionVectorDescriptor): RoaringBitmapArray = - readDeletionVectorOrEmpty(dvStore, dv, tablePath) - val baseBitmap = bitmapOf(currentRemove.deletionVector) - val winningBitmap = bitmapOf(winningAdd.deletionVector) - val currentBitmap = bitmapOf(currentAdd.deletionVector) - - // `baseBitmap` is the DV of `P` at the current txn's read time (carried on its RemoveFile); - // for a 2-way conflict it equals the winner's pre-image too. Both `dv_win` and `dv_cur` are - // supersets of it (a DV only grows), so the newly-deleted rows are `dv \ base` on each side - // and their overlap is `(dv_win INTERSECT dv_cur) MINUS base`. If empty, the two txns - // touched disjoint rows and the schedule `current ; winner` is a valid serialization under - // both WriteSerializable and Serializable (the winner's rewrites/deletes are of rows the - // current txn did not touch), so merging is safe. If non-empty, the same row was touched by - // both -> genuine conflict, left for the standard checks. (For 3+ way chains `base` becomes - // previous winner's DV rather than the original pre-image; the merge stays correct and the - // overlap test stays conservative.) - val newlyDeletedOverlap = winningBitmap.copy() - newlyDeletedOverlap.and(currentBitmap) - newlyDeletedOverlap.andNot(baseBitmap) - - if (newlyDeletedOverlap.isEmpty) { - // Disjoint: merge the deletion vectors and rebase onto the winner's post-image. - val mergedBitmap = winningBitmap.copy() - mergedBitmap.merge(currentBitmap) - val mergedDescriptor = writeMergedDeletionVector(dvStore, tablePath, mergedBitmap) - // Keep the current AddFile's identity (base row ID / default row commit version already - // reconciled by the row-ID phases) but point it at the merged DV. - val rebasedAdd = currentAdd - .copy(deletionVector = mergedDescriptor, dataChange = true) - .withoutTightBoundStats - // Tombstone the winner's now-live AddFile (carries the winning DV) instead of the stale - // pre-image. - val rebasedRemove = winningAdd.removeWithTimestamp() - replacements(path) = (rebasedAdd, rebasedRemove) - rowLevelResolvedPaths += path - } - // else: overlapping row-level modification -> genuine conflict, leave for standard checks. - } - - if (replacements.nonEmpty) { - val newActions = currentTransactionInfo.actions.map { - case a: AddFile if replacements.contains(a.path) => replacements(a.path)._1 - case r: RemoveFile if replacements.contains(r.path) => replacements(r.path)._2 - case other => other - } - // Resolved files are no longer "read" for the purposes of the delete-read check. - val newReadFiles = currentTransactionInfo.readFiles - .filterNot(f => rowLevelResolvedPaths.contains(f.path)) - currentTransactionInfo = - currentTransactionInfo.copy(actions = newActions, readFiles = newReadFiles) - - recordDeltaEvent( - deltaLog, - opType = "delta.rowLevelConcurrency.deletionVectorsMerged", - data = Map( - "winningCommitVersion" -> winningCommitVersion, - "resolvedPaths" -> rowLevelResolvedPaths.size, - "winningOperation" -> winningOperationName.getOrElse("UNKNOWN"))) - } - } - } - - /** Reads a deletion vector into a [[RoaringBitmapArray]], returning an empty bitmap for none. */ - private def readDeletionVectorOrEmpty( - dvStore: DeletionVectorStore, - dv: DeletionVectorDescriptor, - tablePath: Path): RoaringBitmapArray = { - if (dv == null || dv.isEmpty) new RoaringBitmapArray() else dvStore.read(dv, tablePath) - } - - /** - * Persists a merged bitmap to a new deletion vector file and returns its descriptor. - * - * NOTE: this writes a DV file as a side effect of conflict resolution. If the commit ultimately - * fails or is retried against another winning version, the file is orphaned and later reclaimed - * by VACUUM (same lifecycle as any DV written by DML). This mirrors how the DML write path - * persists DVs (see `DeletionVectorWriter.storeSerializedBitmap`). - */ - private def writeMergedDeletionVector( - dvStore: DeletionVectorStore, - tablePath: Path, - bitmap: RoaringBitmapArray): DeletionVectorDescriptor = { - // An empty DV has no on-disk representation (matches DeletionVectorWriter). - if (bitmap.isEmpty) return DeletionVectorDescriptor.EMPTY - val tablePathWithFs = dvStore.pathWithFileSystem(tablePath) - val fileId = UUID.randomUUID() - val writer = dvStore.createWriter(dvStore.generateFileNameInTable(tablePathWithFs, fileId)) - try { - val serialized = DeletionVectorUtils.serialize( - bitmap, RoaringBitmapArrayFormat.Portable, Some(tablePath)) - val range = writer.write(serialized) - DeletionVectorDescriptor.onDiskWithRelativePath( - id = fileId, - sizeInBytes = serialized.length, - cardinality = bitmap.cardinality, - offset = Some(range.offset)) - } finally { - writer.close() - } - } - /** * Check if the new files added by the already committed transactions should have been read by * the current transaction. diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala new file mode 100644 index 00000000000..f343e78bc6e --- /dev/null +++ b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala @@ -0,0 +1,252 @@ +/* + * Copyright (2021) The Delta Lake Project Authors. + * + * 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 org.apache.spark.sql.delta + +import java.util.UUID + +import scala.collection.mutable + +import org.apache.spark.sql.delta.actions.{AddFile, DeletionVectorDescriptor, RemoveFile} +import org.apache.spark.sql.delta.commands.DeletionVectorUtils +import org.apache.spark.sql.delta.deletionvectors.{RoaringBitmapArray, RoaringBitmapArrayFormat} +import org.apache.spark.sql.delta.metering.DeltaLogging +import org.apache.spark.sql.delta.sources.DeltaSQLConf +import org.apache.spark.sql.delta.storage.dv.DeletionVectorStore +import org.apache.hadoop.fs.Path + +/** + * Row-level concurrency resolution for the [[ConflictChecker]]: instead of aborting a concurrent + * DV-based DELETE/UPDATE that touches the same physical files as the winning transaction, MERGE the + * two transactions' deletion vectors when they deleted disjoint rows. + * + * Mixed into [[ConflictChecker]] as a self-typed trait: it reads and rewrites the checker's + * transaction state (`currentTransactionInfo`, `winningCommitSummary`, `deltaLog`, `spark`) + * directly, and lives in its own file so ConflictChecker stays focused on file-level conflict + * detection. The two deletion-vector helpers are `protected` so the OPTIMIZE-vs-DML reconciliation + * (which mixes into the same checker) can reuse them. + */ +trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker => + + /** + * Paths of files whose "same physical file" conflict with the winning transaction was resolved at + * the row level by [[resolveRowLevelConflicts]] (deletion vectors merged). The file-level delete + * and append checks skip these paths, since they have already been reconciled. + */ + protected val rowLevelResolvedPaths = mutable.Set.empty[String] + + /** Whether row-level concurrency resolution is enabled and applicable to this table. */ + protected lazy val rowLevelConcurrencyEnabled: Boolean = + spark.conf.get(DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_ENABLED) && + DeletionVectorUtils.deletionVectorsWritable( + currentTransactionInfo.protocol, currentTransactionInfo.metadata) + + /** The operation name of the winning commit, if available. */ + protected lazy val winningOperationName: Option[String] = + winningCommitSummary.commitInfo.map(_.operation) + + /** + * Whether a file added by the winning transaction can be skipped in the added-files (append) + * conflict check thanks to row-level concurrency resolution. + * + * This is true only when the file's "same physical file" conflict was already reconciled by + * merging deletion vectors ([[resolveRowLevelConflicts]] recorded the path in + * [[rowLevelResolvedPaths]]). In that case the winner's re-added `AddFile(P)` carries the winning + * DV that we already folded into the current transaction's merged DV, so re-checking it would be + * a false conflict. + * + * We deliberately do NOT skip a rewrite-only DML winner's *new image* files here (an UPDATE + * writes updated row values to a fresh path). Those are ordinary non-blind changed-data files + * and can legitimately conflict: e.g. an UPDATE can move a row *into* the loser's predicate + * (winner `SET x = 15`, loser `DELETE WHERE x > 10`, row was `x = 5`), a genuine write-skew that + * the DV union cannot detect. They are arbitrated by the standard added-files check (and, when + * enabled, by conflict-time data skipping over their stats). + */ + protected def canSkipAddedFileForRowLevelConcurrency(addFile: AddFile): Boolean = { + if (!rowLevelConcurrencyEnabled) return false + rowLevelResolvedPaths.contains(addFile.path) + } + + /** + * Resolves "same physical file" conflicts with the winning transaction at the row level. + * + * A DV-based DELETE/UPDATE emits, for each touched file `P`, a `RemoveFile(P)` (tombstone of the + * pre-image) and an `AddFile(P)` carrying a larger deletion vector. When both the winning and the + * current transaction touch the same file `P` this way, the two operations are logically + * independent as long as they mark *different* rows deleted. For every such shared file we: + * 1. decode the winning DV, the current DV and their common base DV (from the pre-image + * `RemoveFile`) as [[RoaringBitmapArray]]s; + * 2. check whether the newly-deleted rows are disjoint, i.e. `(dv_win INTERSECT dv_cur) MINUS + * dv_base` is empty. If they overlap, this is a genuine row-level conflict and we leave the + * file for the standard checks to abort; + * 3. on disjoint sets, merge the DVs (`dv_win UNION dv_cur`), persist a new DV file, + * and rebase the current transaction onto the winner's post-image: the current `AddFile(P)` + * now carries the merged DV and the current `RemoveFile(P)` now tombstones the winner's + * `AddFile(P)`. + * + * Worked example: file `P` holds rows 0..99, undeleted at the current txn's read time + * (`dv_base = {}`). The winner commits `DELETE WHERE id IN (5, 10)` so `dv_win = {5, 10}`; the + * current txn holds `DELETE WHERE id IN (20, 21)` so `dv_cur = {20, 21}`. The overlap is + * `({5,10} INTERSECT {20,21}) MINUS {} = {}` -> disjoint, so we merge to `dv = {5, 10, 20, 21}`, + * point the current `AddFile(P)` at it, and tombstone the winner's `AddFile(P)`; both deletes + * survive. Had the current txn instead deleted row 5 (`dv_cur = {5, 21}`), the overlap would be + * `{5}` -> the same row was deleted twice concurrently, a genuine conflict left for the standard + * checks to abort. (For a 3+ way chain `dv_base` is the previous winner's DV rather than the + * original pre-image, which keeps the overlap test conservative and the merge correct.) + * + * Resolved paths are recorded in [[rowLevelResolvedPaths]] and skipped by the file-level delete + * and append checks. Row identity is preserved for free: the merged file is the same physical + * file, so its base row ID is unchanged and [[reassignOverlappingRowIds]] (already run) leaves it + * alone. Deletion vectors index physical row positions within one immutable Parquet file, so the + * merge needs no row tracking. + */ + protected def resolveRowLevelConflicts(): Unit = { + if (!rowLevelConcurrencyEnabled) return + + // Winning transaction's DV updates: path present in both an AddFile (with a DV) and a + // RemoveFile of the winning commit. + val winningRemovedPaths = winningCommitSummary.removedFiles.map(_.path).toSet + val winningDvUpdates: Map[String, AddFile] = winningCommitSummary.addedFiles.iterator + .filter(a => a.deletionVector != null && winningRemovedPaths.contains(a.path)) + .map(a => a.path -> a) + .toMap + if (winningDvUpdates.isEmpty) return + + // Current transaction's DV updates, indexed by path. + val currentAddByPath = currentTransactionInfo.actions.collect { + case a: AddFile if a.deletionVector != null => a.path -> a + }.toMap + val currentRemoveByPath = currentTransactionInfo.actions.collect { + case r: RemoveFile => r.path -> r + }.toMap + + val sharedPaths = winningDvUpdates.keySet + .intersect(currentAddByPath.keySet) + .intersect(currentRemoveByPath.keySet) + if (sharedPaths.isEmpty) return + + recordTime("resolved-row-level-conflicts") { + val dvStore = DeletionVectorStore.createInstance(deltaLog.newDeltaHadoopConf()) + val tablePath = deltaLog.dataPath + + // path -> (rebased AddFile, rebased RemoveFile) + val replacements = mutable.Map.empty[String, (AddFile, RemoveFile)] + for (path <- sharedPaths) { + val winningAdd = winningDvUpdates(path) + val currentAdd = currentAddByPath(path) + val currentRemove = currentRemoveByPath(path) + + def bitmapOf(dv: DeletionVectorDescriptor): RoaringBitmapArray = + readDeletionVectorOrEmpty(dvStore, dv, tablePath) + val baseBitmap = bitmapOf(currentRemove.deletionVector) + val winningBitmap = bitmapOf(winningAdd.deletionVector) + val currentBitmap = bitmapOf(currentAdd.deletionVector) + + // `baseBitmap` is the DV of `P` at the current txn's read time (carried on its RemoveFile); + // for a 2-way conflict it equals the winner's pre-image too. Both `dv_win` and `dv_cur` are + // supersets of it (a DV only grows), so the newly-deleted rows are `dv \ base` on each side + // and their overlap is `(dv_win INTERSECT dv_cur) MINUS base`. If empty, the two txns + // touched disjoint rows and the schedule `current ; winner` is a valid serialization under + // both WriteSerializable and Serializable (the winner's rewrites/deletes are of rows the + // current txn did not touch), so merging is safe. If non-empty, the same row was touched by + // both -> genuine conflict, left for the standard checks. (For 3+ way chains `base` becomes + // previous winner's DV rather than the original pre-image; the merge stays correct and the + // overlap test stays conservative.) + val newlyDeletedOverlap = winningBitmap.copy() + newlyDeletedOverlap.and(currentBitmap) + newlyDeletedOverlap.andNot(baseBitmap) + + if (newlyDeletedOverlap.isEmpty) { + // Disjoint: merge the deletion vectors and rebase onto the winner's post-image. + val mergedBitmap = winningBitmap.copy() + mergedBitmap.merge(currentBitmap) + val mergedDescriptor = writeMergedDeletionVector(dvStore, tablePath, mergedBitmap) + // Keep the current AddFile's identity (base row ID / default row commit version already + // reconciled by the row-ID phases) but point it at the merged DV. + val rebasedAdd = currentAdd + .copy(deletionVector = mergedDescriptor, dataChange = true) + .withoutTightBoundStats + // Tombstone the winner's now-live AddFile (carries the winning DV) instead of the stale + // pre-image. + val rebasedRemove = winningAdd.removeWithTimestamp() + replacements(path) = (rebasedAdd, rebasedRemove) + rowLevelResolvedPaths += path + } + // else: overlapping row-level modification -> genuine conflict, leave for standard checks. + } + + if (replacements.nonEmpty) { + val newActions = currentTransactionInfo.actions.map { + case a: AddFile if replacements.contains(a.path) => replacements(a.path)._1 + case r: RemoveFile if replacements.contains(r.path) => replacements(r.path)._2 + case other => other + } + // Resolved files are no longer "read" for the purposes of the delete-read check. + val newReadFiles = currentTransactionInfo.readFiles + .filterNot(f => rowLevelResolvedPaths.contains(f.path)) + currentTransactionInfo = + currentTransactionInfo.copy(actions = newActions, readFiles = newReadFiles) + + recordDeltaEvent( + deltaLog, + opType = "delta.rowLevelConcurrency.deletionVectorsMerged", + data = Map( + "winningCommitVersion" -> winningCommitVersion, + "resolvedPaths" -> rowLevelResolvedPaths.size, + "winningOperation" -> winningOperationName.getOrElse("UNKNOWN"))) + } + } + } + + /** Reads a deletion vector into a [[RoaringBitmapArray]], returning an empty bitmap for none. */ + protected def readDeletionVectorOrEmpty( + dvStore: DeletionVectorStore, + dv: DeletionVectorDescriptor, + tablePath: Path): RoaringBitmapArray = { + if (dv == null || dv.isEmpty) new RoaringBitmapArray() else dvStore.read(dv, tablePath) + } + + /** + * Persists a merged bitmap to a new deletion vector file and returns its descriptor. + * + * NOTE: this writes a DV file as a side effect of conflict resolution. If the commit ultimately + * fails or is retried against another winning version, the file is orphaned and later reclaimed + * by VACUUM (same lifecycle as any DV written by DML). This mirrors how the DML write path + * persists DVs (see `DeletionVectorWriter.storeSerializedBitmap`). + */ + protected def writeMergedDeletionVector( + dvStore: DeletionVectorStore, + tablePath: Path, + bitmap: RoaringBitmapArray): DeletionVectorDescriptor = { + // An empty DV has no on-disk representation (matches DeletionVectorWriter). + if (bitmap.isEmpty) return DeletionVectorDescriptor.EMPTY + val tablePathWithFs = dvStore.pathWithFileSystem(tablePath) + val fileId = UUID.randomUUID() + val writer = dvStore.createWriter(dvStore.generateFileNameInTable(tablePathWithFs, fileId)) + try { + val serialized = DeletionVectorUtils.serialize( + bitmap, RoaringBitmapArrayFormat.Portable, Some(tablePath)) + val range = writer.write(serialized) + DeletionVectorDescriptor.onDiskWithRelativePath( + id = fileId, + sizeInBytes = serialized.length, + cardinality = bitmap.cardinality, + offset = Some(range.offset)) + } finally { + writer.close() + } + } +} From 1898a55dc524c2a402f85c6cd61db3a4d4c41ed2 Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Tue, 4 Aug 2026 12:09:27 -0700 Subject: [PATCH 05/16] [Spark] Skip added-file re-scan when no row-level conflict was reconciled canSkipAddedFileForRowLevelConcurrency can only return true for a path in rowLevelResolvedPaths, so when that set is empty the filterNot is a no-op. Guard on rowLevelResolvedPaths.isEmpty to skip the traversal and its allocation on the common no-conflict path. Co-Authored-By: Claude Opus 4.8 --- .../scala/org/apache/spark/sql/delta/ConflictChecker.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala index cf7a0446bdc..9933b68715e 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala @@ -1144,8 +1144,12 @@ private[delta] class ConflictChecker( Seq.empty } + // Only re-scan the added files when row-level resolution actually reconciled something: a + // file is skippable only if its path is in `rowLevelResolvedPaths`, so an empty set means the + // filter is a no-op. Skips the traversal (and its allocation) on the common no-conflict path. val addedFilesAfterRowLevelResolution = - addedFilesToCheckForConflicts.filterNot(canSkipAddedFileForRowLevelConcurrency) + if (rowLevelResolvedPaths.isEmpty) addedFilesToCheckForConflicts + else addedFilesToCheckForConflicts.filterNot(canSkipAddedFileForRowLevelConcurrency) val fileMatchingPartitionReadPredicates = getFirstFileMatchingPartitionPredicates(addedFilesAfterRowLevelResolution) From 6a03f588300f53e3a3ea21c2d15e43ee83a23756 Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Tue, 4 Aug 2026 23:17:42 -0700 Subject: [PATCH 06/16] [Spark] Add MERGE matched-clause tests to RowLevelConcurrencySuite Extend the same-file deletion-vector union coverage to MERGE: - disjoint concurrent MERGE matched-deletes reconcile (DV union, like DELETE) - a MERGE matched-delete reconciles against a concurrent plain DELETE (the union is operation-agnostic) - overlapping MERGE matched-deletes still conflict - a MERGE matched-update (winner) writes an image file, so a concurrent loser conservatively conflicts, mirroring the standalone-UPDATE case MERGE inserts / WHEN NOT MATCHED BY SOURCE stay out of scope (they need per-file row-tracking classification) and are not exercised. Co-Authored-By: Claude Opus 4.8 --- .../sql/delta/RowLevelConcurrencySuite.scala | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala index 6ee50ce2519..47a795da6fb 100644 --- a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -267,6 +267,90 @@ class RowLevelConcurrencySuite extends QueryTest } } + // --------------------------------------------------------------------------- + // MERGE (matched clauses only): a matched-DELETE writes an in-place deletion vector, exactly like + // a standalone DELETE, so disjoint matched-deletes reconcile through the same DV union. A + // matched-UPDATE additionally writes an *image* file (like a standalone UPDATE), so it + // conservatively conflicts. MERGE inserts / WHEN NOT MATCHED BY SOURCE are out of scope here + // (they need per-file row-tracking classification) and are not exercised. + // --------------------------------------------------------------------------- + + /** `MERGE INTO t USING (single-row source) ... ON t.id = s.id` for the given matched action. */ + private def mergeMatched(dir: File, matchId: Long, action: String): String = + s"""MERGE INTO ${tableRef(dir)} t USING (SELECT id FROM range($matchId, ${matchId + 1})) s + |ON t.id = s.id WHEN MATCHED THEN $action""".stripMargin + + test("disjoint concurrent MERGE matched-deletes on the same file both commit by merging DVs") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + val txnA = sqlTxn(mergeMatched(dir, 10, "DELETE"), rowLevelConcurrency = true) + val txnB = sqlTxn(mergeMatched(dir, 20, "DELETE"), rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => id == 10 || id == 20)) + // A matched-DELETE MERGE writes only a DV (no image file), so the two disjoint deletes merge + // into a single surviving file's deletion vector of cardinality 2 -- identical to DELETE. + assert(deletionVectorCardinalities(log) === Seq(2L)) + } + } + + test("MERGE matched-delete reconciles against a concurrent DELETE on the same file") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + // The DV union is op-agnostic: a MERGE matched-delete and a plain DELETE on disjoint rows + // reconcile just like two DELETEs. + val txnA = sqlTxn(mergeMatched(dir, 10, "DELETE"), rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => id == 10 || id == 20)) + assert(deletionVectorCardinalities(log) === Seq(2L)) + } + } + + test("overlapping concurrent MERGE matched-deletes still conflict") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + val txnA = sqlTxn(mergeMatched(dir, 10, "DELETE"), rowLevelConcurrency = true) + val txnB = sqlTxn(mergeMatched(dir, 10, "DELETE"), rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + // Clean abort: only the winner's matched-delete (id=10) is applied; its DV has cardinality 1. + assert(ids(dir) === (0L to 99L).filterNot(_ == 10)) + assert(deletionVectorCardinalities(log) === Seq(1L)) + } + } + + test("DELETE (loser) vs MERGE matched-update (winner): winner image file conservatively " + + "conflicts") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + // A (loser) deletes id=10; B (winner) matched-updates id=20 -> 1020, which masks row 20 with a + // DV and appends an image file for 1020. Like the standalone-UPDATE case above, that image + // file is a non-blind changed-data add the append check cannot prove disjoint, so the loser + // conservatively aborts rather than reconciling. + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(mergeMatched(dir, 20, "UPDATE SET id = 1020"), rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + // Loser aborted cleanly: only the winner's update is applied (row 20 masked, 1020 appended). + assert(ids(dir) === ((0L to 99L).filterNot(_ == 20) :+ 1020L).sorted) + assert(deletionVectorCardinalities(log) === Seq(1L)) + } + } + // --------------------------------------------------------------------------- // Winner fully removes the file -> not reconcilable -> conflict // --------------------------------------------------------------------------- From ccc0f82d7d48220d0bb7b09dbae8b7757464c4e8 Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Fri, 7 Aug 2026 21:14:47 -0700 Subject: [PATCH 07/16] Fail safe on DV resolution errors; single-pass action indexing Apply the same review feedback from Case 1 (#7358) proactively to Case 2: - Fail-safe fallback: extract the per-file DV read/overlap/merge/write into reconcileFileDeletionVectors and wrap the per-path call in try/catch (NonFatal). If resolving one shared file throws (unreadable/corrupt DV, transient I/O), skip row-level resolution for it and let the standard file-level checks abort cleanly with a retryable Concurrent* exception, instead of surfacing an unexpected error out of conflict detection. Other shared files are still resolved independently. - Index the current transaction's AddFile/RemoveFile maps in a single pass over actions instead of two `.collect{}.toMap` traversals. No behavior change on the happy/genuine-conflict paths: RowLevelConcurrencySuite 18/18 still pass. Co-Authored-By: Claude Opus 4.8 --- .../delta/RowLevelConcurrencyResolution.scala | 130 +++++++++++------- 1 file changed, 83 insertions(+), 47 deletions(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala index f343e78bc6e..6c64c6387ea 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala @@ -19,15 +19,19 @@ package org.apache.spark.sql.delta import java.util.UUID import scala.collection.mutable +import scala.util.control.NonFatal import org.apache.spark.sql.delta.actions.{AddFile, DeletionVectorDescriptor, RemoveFile} import org.apache.spark.sql.delta.commands.DeletionVectorUtils import org.apache.spark.sql.delta.deletionvectors.{RoaringBitmapArray, RoaringBitmapArrayFormat} +import org.apache.spark.sql.delta.logging.DeltaLogKeys import org.apache.spark.sql.delta.metering.DeltaLogging import org.apache.spark.sql.delta.sources.DeltaSQLConf import org.apache.spark.sql.delta.storage.dv.DeletionVectorStore import org.apache.hadoop.fs.Path +import org.apache.spark.internal.MDC + /** * Row-level concurrency resolution for the [[ConflictChecker]]: instead of aborting a concurrent * DV-based DELETE/UPDATE that touches the same physical files as the winning transaction, MERGE the @@ -125,13 +129,15 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker .toMap if (winningDvUpdates.isEmpty) return - // Current transaction's DV updates, indexed by path. - val currentAddByPath = currentTransactionInfo.actions.collect { - case a: AddFile if a.deletionVector != null => a.path -> a - }.toMap - val currentRemoveByPath = currentTransactionInfo.actions.collect { - case r: RemoveFile => r.path -> r - }.toMap + // Current transaction's DV updates and file removes, indexed by path. Built in a single pass + // over the actions (last-writer-wins per path, matching the prior `.collect{}.toMap`). + val currentAddByPath = mutable.Map.empty[String, AddFile] + val currentRemoveByPath = mutable.Map.empty[String, RemoveFile] + currentTransactionInfo.actions.foreach { + case a: AddFile if a.deletionVector != null => currentAddByPath(a.path) = a + case r: RemoveFile => currentRemoveByPath(r.path) = r + case _ => + } val sharedPaths = winningDvUpdates.keySet .intersect(currentAddByPath.keySet) @@ -145,47 +151,24 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker // path -> (rebased AddFile, rebased RemoveFile) val replacements = mutable.Map.empty[String, (AddFile, RemoveFile)] for (path <- sharedPaths) { - val winningAdd = winningDvUpdates(path) - val currentAdd = currentAddByPath(path) - val currentRemove = currentRemoveByPath(path) - - def bitmapOf(dv: DeletionVectorDescriptor): RoaringBitmapArray = - readDeletionVectorOrEmpty(dvStore, dv, tablePath) - val baseBitmap = bitmapOf(currentRemove.deletionVector) - val winningBitmap = bitmapOf(winningAdd.deletionVector) - val currentBitmap = bitmapOf(currentAdd.deletionVector) - - // `baseBitmap` is the DV of `P` at the current txn's read time (carried on its RemoveFile); - // for a 2-way conflict it equals the winner's pre-image too. Both `dv_win` and `dv_cur` are - // supersets of it (a DV only grows), so the newly-deleted rows are `dv \ base` on each side - // and their overlap is `(dv_win INTERSECT dv_cur) MINUS base`. If empty, the two txns - // touched disjoint rows and the schedule `current ; winner` is a valid serialization under - // both WriteSerializable and Serializable (the winner's rewrites/deletes are of rows the - // current txn did not touch), so merging is safe. If non-empty, the same row was touched by - // both -> genuine conflict, left for the standard checks. (For 3+ way chains `base` becomes - // previous winner's DV rather than the original pre-image; the merge stays correct and the - // overlap test stays conservative.) - val newlyDeletedOverlap = winningBitmap.copy() - newlyDeletedOverlap.and(currentBitmap) - newlyDeletedOverlap.andNot(baseBitmap) - - if (newlyDeletedOverlap.isEmpty) { - // Disjoint: merge the deletion vectors and rebase onto the winner's post-image. - val mergedBitmap = winningBitmap.copy() - mergedBitmap.merge(currentBitmap) - val mergedDescriptor = writeMergedDeletionVector(dvStore, tablePath, mergedBitmap) - // Keep the current AddFile's identity (base row ID / default row commit version already - // reconciled by the row-ID phases) but point it at the merged DV. - val rebasedAdd = currentAdd - .copy(deletionVector = mergedDescriptor, dataChange = true) - .withoutTightBoundStats - // Tombstone the winner's now-live AddFile (carries the winning DV) instead of the stale - // pre-image. - val rebasedRemove = winningAdd.removeWithTimestamp() - replacements(path) = (rebasedAdd, rebasedRemove) - rowLevelResolvedPaths += path + try { + reconcileFileDeletionVectors( + dvStore, tablePath, + winningDvUpdates(path), currentAddByPath(path), currentRemoveByPath(path)) + .foreach { rebased => + replacements(path) = rebased + rowLevelResolvedPaths += path + } + } catch { + case NonFatal(e) => + // Fail safe: DV decode/merge/write is a pure optimization over the conservative + // default. If it fails for this file (unreadable/corrupt DV, transient I/O), skip + // row-level resolution for it so the standard file-level checks abort cleanly with a + // retryable Concurrent* exception instead of surfacing an unexpected error out of + // conflict detection. Other shared files are still resolved independently. + logWarning(log"Row-level concurrency resolution failed for file " + + log"${MDC(DeltaLogKeys.PATH, path)}; leaving it for the standard conflict checks", e) } - // else: overlapping row-level modification -> genuine conflict, leave for standard checks. } if (replacements.nonEmpty) { @@ -211,6 +194,59 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker } } + /** + * Reconciles the winning and current transactions' deletion vectors for a single shared file `P`. + * Returns the rebased `(AddFile, RemoveFile)` to substitute into the current transaction when the + * two transactions deleted disjoint rows, or `None` when they overlap (a genuine row-level + * conflict, left for the standard checks). All deletion-vector I/O for `P` happens here, so its + * caller can wrap it in a fail-safe boundary. + */ + private def reconcileFileDeletionVectors( + dvStore: DeletionVectorStore, + tablePath: Path, + winningAdd: AddFile, + currentAdd: AddFile, + currentRemove: RemoveFile): Option[(AddFile, RemoveFile)] = { + def bitmapOf(dv: DeletionVectorDescriptor): RoaringBitmapArray = + readDeletionVectorOrEmpty(dvStore, dv, tablePath) + val baseBitmap = bitmapOf(currentRemove.deletionVector) + val winningBitmap = bitmapOf(winningAdd.deletionVector) + val currentBitmap = bitmapOf(currentAdd.deletionVector) + + // `baseBitmap` is the DV of `P` at the current txn's read time (carried on its RemoveFile); + // for a 2-way conflict it equals the winner's pre-image too. Both `dv_win` and `dv_cur` are + // supersets of it (a DV only grows), so the newly-deleted rows are `dv \ base` on each side + // and their overlap is `(dv_win INTERSECT dv_cur) MINUS base`. If empty, the two txns + // touched disjoint rows and the schedule `current ; winner` is a valid serialization under + // both WriteSerializable and Serializable (the winner's rewrites/deletes are of rows the + // current txn did not touch), so merging is safe. If non-empty, the same row was touched by + // both -> genuine conflict, left for the standard checks. (For 3+ way chains `base` becomes + // previous winner's DV rather than the original pre-image; the merge stays correct and the + // overlap test stays conservative.) + val newlyDeletedOverlap = winningBitmap.copy() + newlyDeletedOverlap.and(currentBitmap) + newlyDeletedOverlap.andNot(baseBitmap) + + if (!newlyDeletedOverlap.isEmpty) { + // Overlapping row-level modification -> genuine conflict, leave for the standard checks. + None + } else { + // Disjoint: merge the deletion vectors and rebase onto the winner's post-image. + val mergedBitmap = winningBitmap.copy() + mergedBitmap.merge(currentBitmap) + val mergedDescriptor = writeMergedDeletionVector(dvStore, tablePath, mergedBitmap) + // Keep the current AddFile's identity (base row ID / default row commit version already + // reconciled by the row-ID phases) but point it at the merged DV. + val rebasedAdd = currentAdd + .copy(deletionVector = mergedDescriptor, dataChange = true) + .withoutTightBoundStats + // Tombstone the winner's now-live AddFile (carries the winning DV) instead of the stale + // pre-image. + val rebasedRemove = winningAdd.removeWithTimestamp() + Some((rebasedAdd, rebasedRemove)) + } + } + /** Reads a deletion vector into a [[RoaringBitmapArray]], returning an empty bitmap for none. */ protected def readDeletionVectorOrEmpty( dvStore: DeletionVectorStore, From 64caa4c6b8a8b1a385fd4c3519e97a3cdb6eb05a Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Fri, 7 Aug 2026 22:38:34 -0700 Subject: [PATCH 08/16] [Spark] Parallelize row-level DV reconciliation across shared files resolveRowLevelConflicts reconciled each shared file's deletion vectors sequentially on the caller thread. That per-file work is driver-side object-store DV I/O (read + merge + write), so when a winning transaction conflicts on many files the reconciliation window grows with the file count. Reconcile each shared file independently on a bounded pool (cf. DeltaFileOperations footer reads, which use 8); a single conflicting file stays on the caller thread, so the common case is unchanged. Per-file results are collected and then applied on the caller thread, so there is no shared-state race across the pool. The existing per-file NonFatal fail-safe is preserved (moved into reconcileOnePath): a DV decode/merge/write failure for one file skips row-level resolution for it (the standard checks then abort cleanly) without affecting the others. No behavior change on the happy/genuine-conflict paths. Co-Authored-By: Claude Opus 4.8 --- .../delta/RowLevelConcurrencyResolution.scala | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala index 6c64c6387ea..1c6580ebe1d 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala @@ -31,6 +31,7 @@ import org.apache.spark.sql.delta.storage.dv.DeletionVectorStore import org.apache.hadoop.fs.Path import org.apache.spark.internal.MDC +import org.apache.spark.util.ThreadUtils /** * Row-level concurrency resolution for the [[ConflictChecker]]: instead of aborting a concurrent @@ -148,17 +149,15 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker val dvStore = DeletionVectorStore.createInstance(deltaLog.newDeltaHadoopConf()) val tablePath = deltaLog.dataPath - // path -> (rebased AddFile, rebased RemoveFile) - val replacements = mutable.Map.empty[String, (AddFile, RemoveFile)] - for (path <- sharedPaths) { + // Reconcile each shared file's DVs independently; the work is driver-side object-store DV + // I/O (read + merge + write). When many files conflict, parallelize across a bounded pool + // to shorten the conflict window; a single file stays on the caller thread. + def reconcileOnePath(path: String): Option[(String, (AddFile, RemoveFile))] = try { reconcileFileDeletionVectors( dvStore, tablePath, winningDvUpdates(path), currentAddByPath(path), currentRemoveByPath(path)) - .foreach { rebased => - replacements(path) = rebased - rowLevelResolvedPaths += path - } + .map(path -> _) } catch { case NonFatal(e) => // Fail safe: DV decode/merge/write is a pure optimization over the conservative @@ -168,7 +167,22 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker // conflict detection. Other shared files are still resolved independently. logWarning(log"Row-level concurrency resolution failed for file " + log"${MDC(DeltaLogKeys.PATH, path)}; leaving it for the standard conflict checks", e) + None } + + // Bounded driver parallelism (cf. DeltaFileOperations footer reads, which use 8). + val pathList = sharedPaths.toSeq + val parallelism = math.min(pathList.size, 8) + val reconciled = + if (parallelism <= 1) pathList.flatMap(reconcileOnePath) + else ThreadUtils.parmap(pathList, "rowLevelConflictResolution", parallelism)( + reconcileOnePath).flatten + + // Apply per-file results on the caller thread (no shared-state races across the pool). + val replacements = mutable.Map.empty[String, (AddFile, RemoveFile)] + for ((path, rebased) <- reconciled) { + replacements(path) = rebased + rowLevelResolvedPaths += path } if (replacements.nonEmpty) { From 8af17fd5756b55644b77653239e6d88294e9be6c Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Sat, 8 Aug 2026 07:25:55 -0700 Subject: [PATCH 09/16] [Spark] Test: multi-file row-level DV reconcile exercises the parallel path Every existing RowLevelConcurrencySuite conflict is single-file (sharedPaths.size == 1), which takes the caller-thread branch of resolveRowLevelConflicts. Add a two-file test: a DELETE whose predicate spans two data files DV-updates both in one commit, so the loser reconciles two shared paths at once (parallelism == 2), exercising the ThreadUtils.parmap branch. parmap preserves input order, so the result matches the sequential branch. Co-Authored-By: Claude Opus 4.8 --- .../sql/delta/RowLevelConcurrencySuite.scala | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala index 47a795da6fb..9d7d7c55ba1 100644 --- a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -422,6 +422,42 @@ class RowLevelConcurrencySuite extends QueryTest } } + // --------------------------------------------------------------------------- + // Two shared files in one conflict -> bounded-parallel reconcile (ThreadUtils.parmap). + // Every test above conflicts on one file (sharedPaths.size == 1 -> caller-thread branch). + // A DELETE whose predicate spans two files DV-updates both in ONE commit, so the loser + // reconciles two shared paths at once, crossing the `parallelism <= 1` boundary into parmap. + // parmap preserves input order, so the reconciled result matches the sequential branch. + // --------------------------------------------------------------------------- + + test("disjoint DELETEs spanning two files reconcile via the bounded-parallel path") { + withTempDir { dir => + // Two data files: ids [0,100) and [100,200), one file each (separate appends). + spark.range(start = 0, end = 100, step = 1, numPartitions = 1) + .write.format("delta").mode("append").save(dir.getAbsolutePath) + spark.range(start = 100, end = 200, step = 1, numPartitions = 1) + .write.format("delta").mode("append").save(dir.getAbsolutePath) + val log = DeltaLog.forTable(spark, dir.getCanonicalPath) + assert(log.update().allFiles.collect().length === 2, "test table must have two data files") + + // Each DELETE's IN list spans both files (one id < 100, one id >= 100), so each commit + // removes and re-adds BOTH files with a per-file DV. The loser therefore has two shared + // paths in one resolveRowLevelConflicts call -> parallelism == 2 (the parmap branch). + val txnA = + sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id IN (20, 120)", rowLevelConcurrency = true) + val txnB = + sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id IN (10, 110)", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === (0L to 199L).filterNot(id => Set(10L, 20L, 110L, 120L).contains(id))) + // Both files survive, each carrying its own merged DV (winner + loser delete): cardinality 2. + assert(deletionVectorCardinalities(log).sorted === Seq(2L, 2L)) + } + } + // --------------------------------------------------------------------------- // Partitioned table (DV merge is partition-agnostic) // --------------------------------------------------------------------------- From e9370cffffeb7ea5f3c5fbffa6b41aef26439b5c Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Wed, 12 Aug 2026 23:23:38 -0700 Subject: [PATCH 10/16] [Spark] Document N-way DV-merge correctness; add base-DV chain and Serializable read-op tests Expand reconcileFileDeletionVectors' comment into an explicit induction on the winner chain: after merging winner k the current txn's base advances to winner k's DV, so each step's overlap test reduces to the two ops' own new deletes -- precise for any number of winners and independent of winner order. Add two RowLevelConcurrencySuite tests: - three disjoint DELETEs reconciling over a non-empty base DV (the deepest chain the ordering helpers reach; combines the base-DV and N-way subtleties). - a Serializable MERGE matched-delete loser (a read-then-modify op) vs a concurrent disjoint DELETE, confirming reconciled paths drop from readFiles under the stricter isolation. Co-Authored-By: Claude Opus 4.8 --- .../delta/RowLevelConcurrencyResolution.scala | 29 +++++++---- .../sql/delta/RowLevelConcurrencySuite.scala | 52 +++++++++++++++++++ 2 files changed, 71 insertions(+), 10 deletions(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala index 1c6580ebe1d..a9e5a82e8c2 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala @@ -227,16 +227,25 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker val winningBitmap = bitmapOf(winningAdd.deletionVector) val currentBitmap = bitmapOf(currentAdd.deletionVector) - // `baseBitmap` is the DV of `P` at the current txn's read time (carried on its RemoveFile); - // for a 2-way conflict it equals the winner's pre-image too. Both `dv_win` and `dv_cur` are - // supersets of it (a DV only grows), so the newly-deleted rows are `dv \ base` on each side - // and their overlap is `(dv_win INTERSECT dv_cur) MINUS base`. If empty, the two txns - // touched disjoint rows and the schedule `current ; winner` is a valid serialization under - // both WriteSerializable and Serializable (the winner's rewrites/deletes are of rows the - // current txn did not touch), so merging is safe. If non-empty, the same row was touched by - // both -> genuine conflict, left for the standard checks. (For 3+ way chains `base` becomes - // previous winner's DV rather than the original pre-image; the merge stays correct and the - // overlap test stays conservative.) + // `baseBitmap` is the DV of `P` at the current txn's read time (carried on its RemoveFile). + // Both `dv_win` and `dv_cur` are supersets of it (a DV only grows), so each side's *newly* + // deleted rows are `dv \ base`, and their overlap is `(dv_win INTERSECT dv_cur) MINUS base`. + // Empty overlap => the two txns deleted disjoint rows, and `current ; winner` is a valid + // serialization under both WriteSerializable and Serializable (each deleted only rows the + // other did not touch), so the DV union is safe. Non-empty => the same row was deleted + // concurrently: a genuine conflict, left for the standard checks to abort. + // + // N-way: the current txn reconciles against a chain of winners, once per winning commit. + // Induction on the number of prior winners `k` already merged into the current txn: + // k = 0 (first winner): `base` is P's read-time DV, so `dv_cur \ base` and `dv_win \ base` + // are exactly the two txns' own new deletes -- the test is precise. + // k -> k+1: merging winner k rebased the current RemoveFile onto winner k's AddFile (so now + // `base = dv_win_k`) and the current AddFile onto the merged DV. Winner k+1 committed on + // top of winner k, so `dv_win_k` is a subset of `dv_win_{k+1}`. The test + // `(dv_win_{k+1} INTERSECT dv_cur) MINUS dv_win_k` then reduces to winner k+1's *own* new + // deletes intersected with the current txn's own new deletes -- prior winners cancel out. + // Every step compares only the two operations' genuinely new rows, and the merged DV is a + // union of disjoint contributions, so the result is independent of winner order. val newlyDeletedOverlap = winningBitmap.copy() newlyDeletedOverlap.and(currentBitmap) newlyDeletedOverlap.andNot(baseBitmap) diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala index 9d7d7c55ba1..edc43929b81 100644 --- a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -167,6 +167,29 @@ class RowLevelConcurrencySuite extends QueryTest } } + test("Serializable: a MERGE matched-delete (read-then-modify) reconciles vs a concurrent DELETE") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + sql(s"ALTER TABLE ${tableRef(dir)} SET TBLPROPERTIES " + + s"('${DeltaConfigs.ISOLATION_LEVEL.key}' = 'Serializable')") + // The disjoint-DELETE Serializable test above already covers a delete-only loser. Here the + // loser is a MERGE, which *scans* the file to evaluate its ON condition, so the shared path + // enters the current txn's readFiles as a genuine read (not merely as a delete target). The + // winner's disjoint DELETE removes that same file, which under Serializable would trip the + // delete-read check -- unless row-level resolution drops the reconciled path from readFiles. + // This confirms that removal holds for a read-then-modify op under the stricter isolation. + val txnA = sqlTxn(mergeMatched(dir, 10, "DELETE"), rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => id == 10 || id == 20)) + assert(deletionVectorCardinalities(log) === Seq(2L)) + } + } + test("overlapping concurrent DELETEs still conflict") { withTempDir { dir => val log = createSingleFileTableWithDVs(dir) @@ -422,6 +445,35 @@ class RowLevelConcurrencySuite extends QueryTest } } + test("three concurrent disjoint DELETEs reconcile on a file that already carries a base DV") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + // Establish a non-empty base DV before any concurrency: delete id=5 (file DV cardinality 1). + // All three concurrent txns read this DV as their common base, and the last one (A) then + // reconciles against a two-deep winner chain (B, then C) stacked on that base. This is the + // deepest chain the ordering helpers reach, and it combines the base-DV subtlety with the + // N-way induction: each step must subtract the *prior winner's* DV, not the read-time base, + // or the accumulated deletes would look like a false overlap. + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 5") + assert(deletionVectorCardinalities(log) === Seq(1L)) + + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + val txnC = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 30", rowLevelConcurrency = true) + + // A starts; B commits; C commits (reading B's state); A commits last (reconciles vs B, C). + val (futureA, futureB, futureC) = + runTxnsWithOrder__A_Start__B__C__A_End(txnA, txnB, txnC) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + ThreadUtils.awaitResult(futureC, Duration.Inf) + + assert(ids(dir) === (0L to 99L).filterNot(id => Set(5L, 10L, 20L, 30L).contains(id))) + // Base row 5 plus three disjoint concurrent deletes -> merged deletion vector cardinality 4. + assert(deletionVectorCardinalities(log) === Seq(4L)) + } + } + // --------------------------------------------------------------------------- // Two shared files in one conflict -> bounded-parallel reconcile (ThreadUtils.parmap). // Every test above conflicts on one file (sharedPaths.size == 1 -> caller-thread branch). From 0b8f4923428612bbd5b7467333fbaea67265dd4f Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Wed, 12 Aug 2026 23:38:09 -0700 Subject: [PATCH 11/16] [Spark] Simplify RLC integration: prune the winning summary instead of guarding each check Row-level resolution previously threaded a `!rowLevelResolvedPaths.contains(...)` guard into four separate file-level checks (append, delete-read, whole-table, delete-delete) in ConflictChecker. Instead, rewrite both sides of the conflict once in resolveRowLevelConflicts: rebase the current transaction (as before) and prune the reconciled AddFile(P)/RemoveFile(P) pair from a single effective `winningCommitSummary` (now a `var`, mirroring `currentTransactionInfo`). The four checks revert to byte-identical-to-upstream, since they now see a winner that never touched the reconciled files. Removes the trait-level mutable `rowLevelResolvedPaths` set and `canSkipAddedFileForRowLevelConcurrency`; the "don't skip UPDATE image files" reasoning moves to pruneReconciledFiles (only the same-path pair is pruned). ConflictChecker footprint drops to a mixin + one `var` + one call site; all subtle DV logic stays confined to reconcileFileDeletionVectors. RowLevelConcurrencySuite 21/21, FeatureEnablementConcurrencySuite 40/40. Co-Authored-By: Claude Opus 4.8 --- .../spark/sql/delta/ConflictChecker.scala | 25 +++--- .../delta/RowLevelConcurrencyResolution.scala | 77 +++++++++---------- 2 files changed, 47 insertions(+), 55 deletions(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala index 9933b68715e..623b14b4866 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala @@ -218,7 +218,11 @@ object WinningCommitSummary { private[delta] class ConflictChecker( protected val spark: SparkSession, initialCurrentTransactionInfo: CurrentTransactionInfo, - protected val winningCommitSummary: WinningCommitSummary, + // A `var` (reassigned once by [[resolveRowLevelConflicts]]) to drop files that were reconciled + // at the row level, mirroring how `currentTransactionInfo` is rebased. After that the + // file-level checks run against a summary that never mentions the reconciled files, so they + // need no changes. + protected var winningCommitSummary: WinningCommitSummary, isolationLevel: IsolationLevel) extends DeltaLogging with ConflictCheckerPredicateElimination with RowLevelConcurrencyResolution { @@ -1144,15 +1148,8 @@ private[delta] class ConflictChecker( Seq.empty } - // Only re-scan the added files when row-level resolution actually reconciled something: a - // file is skippable only if its path is in `rowLevelResolvedPaths`, so an empty set means the - // filter is a no-op. Skips the traversal (and its allocation) on the common no-conflict path. - val addedFilesAfterRowLevelResolution = - if (rowLevelResolvedPaths.isEmpty) addedFilesToCheckForConflicts - else addedFilesToCheckForConflicts.filterNot(canSkipAddedFileForRowLevelConcurrency) - val fileMatchingPartitionReadPredicates = - getFirstFileMatchingPartitionPredicates(addedFilesAfterRowLevelResolution) + getFirstFileMatchingPartitionPredicates(addedFilesToCheckForConflicts) if (fileMatchingPartitionReadPredicates.nonEmpty) { throw DeltaErrors.concurrentAppendException( @@ -1174,7 +1171,7 @@ private[delta] class ConflictChecker( val readFilePaths = currentTransactionInfo.readFiles.map( f => f.path -> f.partitionValues).toMap val deleteReadOverlap = winningCommitSummary.removedFiles - .find(r => readFilePaths.contains(r.path) && !rowLevelResolvedPaths.contains(r.path)) + .find(r => readFilePaths.contains(r.path)) if (deleteReadOverlap.nonEmpty) { val partitionOpt = getPrettyPartitionMessage(readFilePaths(deleteReadOverlap.get.path)) throw DeltaErrors.concurrentDeleteReadException( @@ -1183,11 +1180,7 @@ private[delta] class ConflictChecker( winningCommitVersion, partitionOpt) } - // Row-level concurrency: a removed file that was reconciled at the row level must not - // re-trigger the whole-table conflict either. - val unresolvedRemovedFiles = - winningCommitSummary.removedFiles.exists(r => !rowLevelResolvedPaths.contains(r.path)) - if (unresolvedRemovedFiles && currentTransactionInfo.readWholeTable) { + if (winningCommitSummary.removedFiles.nonEmpty && currentTransactionInfo.readWholeTable) { throw DeltaErrors.concurrentDeleteReadException( winningCommitSummary.commitInfo, getTableNameOrPath, @@ -1208,7 +1201,7 @@ private[delta] class ConflictChecker( .collect { case r: RemoveFile => r.path -> r.partitionValues } .toMap val deleteOverlap = winningCommitSummary.removedFiles - .find(r => deletedFilePaths.contains(r.path) && !rowLevelResolvedPaths.contains(r.path)) + .find(r => deletedFilePaths.contains(r.path)) if (deleteOverlap.nonEmpty) { val partitionOpt = getPrettyPartitionMessage(deletedFilePaths(deleteOverlap.get.path)) throw DeltaErrors.concurrentDeleteDeleteException( diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala index a9e5a82e8c2..84f40e32911 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala @@ -46,13 +46,6 @@ import org.apache.spark.util.ThreadUtils */ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker => - /** - * Paths of files whose "same physical file" conflict with the winning transaction was resolved at - * the row level by [[resolveRowLevelConflicts]] (deletion vectors merged). The file-level delete - * and append checks skip these paths, since they have already been reconciled. - */ - protected val rowLevelResolvedPaths = mutable.Set.empty[String] - /** Whether row-level concurrency resolution is enabled and applicable to this table. */ protected lazy val rowLevelConcurrencyEnabled: Boolean = spark.conf.get(DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_ENABLED) && @@ -63,28 +56,6 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker protected lazy val winningOperationName: Option[String] = winningCommitSummary.commitInfo.map(_.operation) - /** - * Whether a file added by the winning transaction can be skipped in the added-files (append) - * conflict check thanks to row-level concurrency resolution. - * - * This is true only when the file's "same physical file" conflict was already reconciled by - * merging deletion vectors ([[resolveRowLevelConflicts]] recorded the path in - * [[rowLevelResolvedPaths]]). In that case the winner's re-added `AddFile(P)` carries the winning - * DV that we already folded into the current transaction's merged DV, so re-checking it would be - * a false conflict. - * - * We deliberately do NOT skip a rewrite-only DML winner's *new image* files here (an UPDATE - * writes updated row values to a fresh path). Those are ordinary non-blind changed-data files - * and can legitimately conflict: e.g. an UPDATE can move a row *into* the loser's predicate - * (winner `SET x = 15`, loser `DELETE WHERE x > 10`, row was `x = 5`), a genuine write-skew that - * the DV union cannot detect. They are arbitrated by the standard added-files check (and, when - * enabled, by conflict-time data skipping over their stats). - */ - protected def canSkipAddedFileForRowLevelConcurrency(addFile: AddFile): Boolean = { - if (!rowLevelConcurrencyEnabled) return false - rowLevelResolvedPaths.contains(addFile.path) - } - /** * Resolves "same physical file" conflicts with the winning transaction at the row level. * @@ -179,35 +150,63 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker reconcileOnePath).flatten // Apply per-file results on the caller thread (no shared-state races across the pool). - val replacements = mutable.Map.empty[String, (AddFile, RemoveFile)] - for ((path, rebased) <- reconciled) { - replacements(path) = rebased - rowLevelResolvedPaths += path - } - + val replacements = reconciled.toMap if (replacements.nonEmpty) { + val resolvedPaths = replacements.keySet + + // Rewrite BOTH sides of the conflict so the residual is a plain no-conflict state that the + // file-level checks (unchanged from upstream) handle. Current txn: rebase each AddFile(P) + // onto the merged DV and each RemoveFile(P) onto the winner's post-image, and drop P from + // readFiles (it is no longer "read" for the delete-read check). val newActions = currentTransactionInfo.actions.map { case a: AddFile if replacements.contains(a.path) => replacements(a.path)._1 case r: RemoveFile if replacements.contains(r.path) => replacements(r.path)._2 case other => other } - // Resolved files are no longer "read" for the purposes of the delete-read check. - val newReadFiles = currentTransactionInfo.readFiles - .filterNot(f => rowLevelResolvedPaths.contains(f.path)) + val newReadFiles = + currentTransactionInfo.readFiles.filterNot(f => resolvedPaths.contains(f.path)) currentTransactionInfo = currentTransactionInfo.copy(actions = newActions, readFiles = newReadFiles) + // Winning side: drop the reconciled AddFile(P)/RemoveFile(P) pair from the summary, so no + // check sees P as a winner-side add or remove. + winningCommitSummary = pruneReconciledFiles(winningCommitSummary, resolvedPaths) recordDeltaEvent( deltaLog, opType = "delta.rowLevelConcurrency.deletionVectorsMerged", data = Map( "winningCommitVersion" -> winningCommitVersion, - "resolvedPaths" -> rowLevelResolvedPaths.size, + "resolvedPaths" -> resolvedPaths.size, "winningOperation" -> winningOperationName.getOrElse("UNKNOWN"))) } } } + /** + * Returns a copy of `summary` with the reconciled files removed: for each resolved path we drop + * the winner's `AddFile(P)` (its deletion vector was folded into the current transaction's merged + * DV) and its `RemoveFile(P)` (tombstone of the shared pre-image). Rebuilding from the filtered + * action list recomputes the derived views (`addedFiles`, `removedFiles`, + * `changedDataAddedFiles`, ...), so the file-level conflict checks see a winner that never + * touched these files. + * + * Only the same-path reconciled pair is pruned. A rewrite-only DML winner's *new image* files (an + * UPDATE writes updated row values to a fresh path) are left in the summary and arbitrated by the + * standard added-files check: an UPDATE can move a row *into* the loser's predicate (winner + * `SET x = 15`, loser `DELETE WHERE x > 10`, row was `x = 5`), a genuine write-skew the DV union + * cannot detect. All non-file actions (protocol, metadata, domain metadata) are preserved. + */ + private def pruneReconciledFiles( + summary: WinningCommitSummary, + resolvedPaths: Set[String]): WinningCommitSummary = { + val prunedActions = summary.actions.filterNot { + case a: AddFile => resolvedPaths.contains(a.path) + case r: RemoveFile => resolvedPaths.contains(r.path) + case _ => false + } + new WinningCommitSummary(prunedActions, summary.fileStatus, summary.readTimeMs) + } + /** * Reconciles the winning and current transactions' deletion vectors for a single shared file `P`. * Returns the rebased `(AddFile, RemoveFile)` to substitute into the current transaction when the From 23a8c77dcbe74ebb72c0842bcb2a97f6ab413111 Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Wed, 14 Oct 2026 22:38:12 -0700 Subject: [PATCH 12/16] Only reconcile a shared file when the current txn's reads are unaffected Reconciling a shared file hides the winner's deletes in it from the delete-read check. Require that none of the rows the winner newly deleted match the current transaction's read predicates (one signed-weight scan of the base-DV and winner-DV images), refuse whole-table reads, and accept a MERGE without a scan only when it has no NOT MATCHED clause or has an unconditional MATCHED clause. Previously a MERGE with a conditional matched clause and an insert could commit after a winner deleted a row it matched but left alone, skipping the insert that row's deletion requires. Merged DV files are now written only once the whole resolution is known to succeed, and the deletionVectorsMerged event is recorded after all checks against the winning commit pass. Co-Authored-By: Claude Opus 5.5 --- .../spark/sql/delta/ConflictChecker.scala | 1 + .../delta/RowLevelConcurrencyResolution.scala | 249 ++++++++++++++---- .../sql/delta/RowLevelConcurrencySuite.scala | 130 ++++++++- 3 files changed, 320 insertions(+), 60 deletions(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala index 623b14b4866..e319f0d9382 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala @@ -317,6 +317,7 @@ private[delta] class ConflictChecker( checkForDeletedFilesAgainstCurrentTxnDeletedFiles() resolveTimestampOrderingConflicts() + recordRowLevelConflictResolution() logMetrics() currentTransactionInfo } diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala index 84f40e32911..854ce585ba1 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala @@ -21,6 +21,7 @@ import java.util.UUID import scala.collection.mutable import scala.util.control.NonFatal +import org.apache.spark.sql.delta.ClassicColumnConversions._ import org.apache.spark.sql.delta.actions.{AddFile, DeletionVectorDescriptor, RemoveFile} import org.apache.spark.sql.delta.commands.DeletionVectorUtils import org.apache.spark.sql.delta.deletionvectors.{RoaringBitmapArray, RoaringBitmapArrayFormat} @@ -31,6 +32,9 @@ import org.apache.spark.sql.delta.storage.dv.DeletionVectorStore import org.apache.hadoop.fs.Path import org.apache.spark.internal.MDC +import org.apache.spark.sql.{Column, DataFrame} +import org.apache.spark.sql.catalyst.expressions.{AttributeReference, Expression} +import org.apache.spark.sql.functions.{coalesce, col, lit, sum} import org.apache.spark.util.ThreadUtils /** @@ -83,6 +87,17 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker * checks to abort. (For a 3+ way chain `dv_base` is the previous winner's DV rather than the * original pre-image, which keeps the overlap test conservative and the merge correct.) * + * Reconciling `P` hides the winner's deletes in `P` from the delete-read check, so a file is only + * reconciled when the current transaction's reads are provably unaffected by them: none of the + * rows the winner newly deleted may match any of the current transaction's read predicates (see + * [[currentReadsUnaffectedByWinnerDeletes]]). The current transaction then produces the same + * actions whether it runs before or after the winner, so `winner ; current` -- the commit order + * -- is a valid serialization. For a DELETE/UPDATE the only read is its own condition, whose + * matching rows in `P` are exactly the rows it deleted, so the disjointness test above already + * implies this. A MERGE, or any transaction that also read other rows, can see a winner's delete: + * `MERGE ... WHEN MATCHED AND cond THEN DELETE WHEN NOT MATCHED THEN INSERT` that matched a row + * without deleting it must INSERT the source row once the winner deletes that target row. + * * Resolved paths are recorded in [[rowLevelResolvedPaths]] and skipped by the file-level delete * and append checks. Row identity is preserved for free: the merged file is the same physical * file, so its base row ID is unchanged and [[reassignOverlappingRowIds]] (already run) leaves it @@ -91,6 +106,8 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker */ protected def resolveRowLevelConflicts(): Unit = { if (!rowLevelConcurrencyEnabled) return + // A whole-table read observes every row, so any row the winner deleted affects it. + if (currentTransactionInfo.readWholeTable) return // Winning transaction's DV updates: path present in both an AddFile (with a DV) and a // RemoveFile of the winning commit. @@ -120,34 +137,47 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker val dvStore = DeletionVectorStore.createInstance(deltaLog.newDeltaHadoopConf()) val tablePath = deltaLog.dataPath - // Reconcile each shared file's DVs independently; the work is driver-side object-store DV - // I/O (read + merge + write). When many files conflict, parallelize across a bounded pool - // to shorten the conflict window; a single file stays on the caller thread. - def reconcileOnePath(path: String): Option[(String, (AddFile, RemoveFile))] = - try { - reconcileFileDeletionVectors( - dvStore, tablePath, - winningDvUpdates(path), currentAddByPath(path), currentRemoveByPath(path)) - .map(path -> _) - } catch { + // Each shared file's DVs are decoded and checked for disjointness, and later merged and + // written, independently; the work is driver-side object-store DV I/O. When many files + // conflict, parallelize across a bounded pool to shorten the conflict window; a single file + // stays on the caller thread. + def forEachPath[A, B](items: Seq[A], name: String)(f: A => Option[B]): Seq[B] = { + // Bounded driver parallelism (cf. DeltaFileOperations footer reads, which use 8). + val parallelism = math.min(items.size, 8) + if (parallelism <= 1) items.flatMap(f) + else ThreadUtils.parmap(items, name, parallelism)(f).flatten + } + // Fail safe: DV decode/merge/write is a pure optimization over the conservative default. If + // it fails for a file (unreadable/corrupt DV, transient I/O), skip row-level resolution for + // it so the standard file-level checks abort cleanly with a retryable Concurrent* exception + // instead of surfacing an unexpected error out of conflict detection. + def failSafe[B](path: String)(f: => Option[B]): Option[B] = + try f catch { case NonFatal(e) => - // Fail safe: DV decode/merge/write is a pure optimization over the conservative - // default. If it fails for this file (unreadable/corrupt DV, transient I/O), skip - // row-level resolution for it so the standard file-level checks abort cleanly with a - // retryable Concurrent* exception instead of surfacing an unexpected error out of - // conflict detection. Other shared files are still resolved independently. logWarning(log"Row-level concurrency resolution failed for file " + log"${MDC(DeltaLogKeys.PATH, path)}; leaving it for the standard conflict checks", e) None } - // Bounded driver parallelism (cf. DeltaFileOperations footer reads, which use 8). - val pathList = sharedPaths.toSeq - val parallelism = math.min(pathList.size, 8) - val reconciled = - if (parallelism <= 1) pathList.flatMap(reconcileOnePath) - else ThreadUtils.parmap(pathList, "rowLevelConflictResolution", parallelism)( - reconcileOnePath).flatten + val disjointFiles = forEachPath(sharedPaths.toSeq, "rowLevelConflictCheck") { path => + failSafe(path) { + disjointFileDeletionVectors( + dvStore, tablePath, + winningDvUpdates(path), currentAddByPath(path), currentRemoveByPath(path)) + } + } + // A file the winner and current txn both deleted rows from cannot be reconciled, and leaves a + // conflict for the standard checks anyway. Likewise if the winner's deletes are visible to + // the current txn's reads. Either way nothing is merged, so no DV file is written. + val reconcilable = disjointFiles.size == sharedPaths.size && + currentReadsUnaffectedByWinnerDeletes(disjointFiles) + val reconciled = if (!reconcilable) Seq.empty else { + forEachPath(disjointFiles, "rowLevelConflictResolution") { file => + failSafe(file.currentAdd.path) { + Some(file.currentAdd.path -> mergeFileDeletionVectors(dvStore, tablePath, file)) + } + } + } // Apply per-file results on the caller thread (no shared-state races across the pool). val replacements = reconciled.toMap @@ -170,18 +200,105 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker // Winning side: drop the reconciled AddFile(P)/RemoveFile(P) pair from the summary, so no // check sees P as a winner-side add or remove. winningCommitSummary = pruneReconciledFiles(winningCommitSummary, resolvedPaths) + numRowLevelResolvedPaths = resolvedPaths.size + } + } + } + + /** Number of files reconciled by [[resolveRowLevelConflicts]] against this winning commit. */ + private var numRowLevelResolvedPaths = 0 + + /** + * Records the row-level resolution, if any. Called once every check against the winning commit + * has passed, so a reconciliation undone by a later conflict is not reported as a merge. + */ + protected def recordRowLevelConflictResolution(): Unit = { + if (numRowLevelResolvedPaths == 0) return + recordDeltaEvent( + deltaLog, + opType = "delta.rowLevelConcurrency.deletionVectorsMerged", + data = Map( + "winningCommitVersion" -> winningCommitVersion, + "resolvedPaths" -> numRowLevelResolvedPaths, + "winningOperation" -> winningOperationName.getOrElse("UNKNOWN"))) + } - recordDeltaEvent( - deltaLog, - opType = "delta.rowLevelConcurrency.deletionVectorsMerged", - data = Map( - "winningCommitVersion" -> winningCommitVersion, - "resolvedPaths" -> resolvedPaths.size, - "winningOperation" -> winningOperationName.getOrElse("UNKNOWN"))) + /** + * Whether none of the rows the winner newly deleted from `files` match any of the current + * transaction's read predicates, i.e. its reads return the same rows before and after the + * winner's deletes. + * + * The winner's newly-deleted rows of a file are its live rows under the current transaction's + * base DV minus its live rows under the winner's DV (the latter is a subset), so for the read + * condition `cond` (OR across reads, AND within a read): + * + * matches(newly deleted) = matches(base-DV live rows) - matches(winner-DV live rows) + * + * Both images of all files are scanned with signed weights (+1 base, -1 winner) and summed in a + * single Spark job. Conservative: a read predicate is evaluated on its deterministic, + * subquery-free conjuncts (dropping a conjunct only matches more rows), a read with none of + * those matches every row, and any failure counts as affected. + * + * A MERGE's target read usually matches every row, but most MERGEs are provably unaffected + * without a scan, see [[mergeUnaffectedByDeletesOfUntouchedRows]]. + */ + private def currentReadsUnaffectedByWinnerDeletes(files: Seq[DisjointFile]): Boolean = { + currentTransactionInfo.op match { + case merge: DeltaOperations.Merge if mergeUnaffectedByDeletesOfUntouchedRows(merge) => + return true + case _ => + } + val readPredicates = currentTransactionInfo.readPredicates + if (readPredicates.isEmpty) return false + val conjunctsPerRead = readPredicates.map { rp => + (rp.partitionPredicates ++ rp.dataPredicates).filter { e => + e.deterministic && !DeltaTableUtils.containsSubquery(e) } } + if (conjunctsPerRead.exists(_.isEmpty)) return false + try { + val snapshot = currentTransactionInfo.readSnapshot + def matchingRowWeights(images: Seq[AddFile], weight: Long): DataFrame = { + val df = deltaLog.createDataFrame( + snapshot, images, actionTypeOpt = Some("rowLevelConcurrencyReadCheck")) + val condition = conjunctsPerRead + .map(_.map(e => rebindToDataFrame(e, df)).reduce(_ && _)) + .reduce(_ || _) + df.where(condition).select(lit(weight).as("weight")) + } + val baseImages = files.map(f => f.currentAdd.copy(deletionVector = f.baseDeletionVector)) + val winnerImages = files.map(_.winningAdd) + val newlyDeletedMatches = matchingRowWeights(baseImages, 1L) + .union(matchingRowWeights(winnerImages, -1L)) + .select(coalesce(sum(col("weight")), lit(0L))) + .head() + .getLong(0) + newlyDeletedMatches == 0 + } catch { + case NonFatal(e) => + logWarning(log"Row-level concurrency read check failed; leaving the shared files for " + + log"the standard conflict checks", e) + false + } } + /** + * Whether a MERGE's outcome is unchanged when rows it did not modify are deleted first. Each + * target row's fate depends only on that row and the source: a row that matched a MATCHED (or + * NOT MATCHED BY SOURCE) clause was modified, so it is in the MERGE's own new deletes and + * disjointness already excludes it from the winner's. The exception is NOT MATCHED: a source + * row is inserted only if NO target row matched it, so deleting a matched-but-unmodified target + * row changes the inserts. That can only happen when no MATCHED clause is unconditional. + */ + private def mergeUnaffectedByDeletesOfUntouchedRows(merge: DeltaOperations.Merge): Boolean = + merge.notMatchedPredicates.isEmpty || merge.matchedPredicates.exists(_.predicate.isEmpty) + + /** Rebinds `e`'s attributes to `df`'s columns by name. */ + private def rebindToDataFrame(e: Expression, df: DataFrame): Column = + Column(e.transformUp { + case a: AttributeReference => df.col("`" + a.name.replace("`", "``") + "`").expr + }) + /** * Returns a copy of `summary` with the reconciled files removed: for each resolved path we drop * the winner's `AddFile(P)` (its deletion vector was folded into the current transaction's merged @@ -208,31 +325,39 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker } /** - * Reconciles the winning and current transactions' deletion vectors for a single shared file `P`. - * Returns the rebased `(AddFile, RemoveFile)` to substitute into the current transaction when the - * two transactions deleted disjoint rows, or `None` when they overlap (a genuine row-level - * conflict, left for the standard checks). All deletion-vector I/O for `P` happens here, so its - * caller can wrap it in a fail-safe boundary. + * A shared file `P` whose newly-deleted rows are disjoint between the winning and current + * transactions, with the decoded DVs needed to merge them. */ - private def reconcileFileDeletionVectors( + private case class DisjointFile( + winningAdd: AddFile, + currentAdd: AddFile, + baseDeletionVector: DeletionVectorDescriptor, + winningBitmap: RoaringBitmapArray, + currentBitmap: RoaringBitmapArray) + + /** + * Decodes the winning, current and base deletion vectors of a single shared file `P` and returns + * them when the two transactions deleted disjoint rows, or `None` when they overlap (a genuine + * row-level conflict, left for the standard checks). + */ + private def disjointFileDeletionVectors( dvStore: DeletionVectorStore, tablePath: Path, winningAdd: AddFile, currentAdd: AddFile, - currentRemove: RemoveFile): Option[(AddFile, RemoveFile)] = { + currentRemove: RemoveFile): Option[DisjointFile] = { def bitmapOf(dv: DeletionVectorDescriptor): RoaringBitmapArray = readDeletionVectorOrEmpty(dvStore, dv, tablePath) val baseBitmap = bitmapOf(currentRemove.deletionVector) val winningBitmap = bitmapOf(winningAdd.deletionVector) val currentBitmap = bitmapOf(currentAdd.deletionVector) - // `baseBitmap` is the DV of `P` at the current txn's read time (carried on its RemoveFile). // Both `dv_win` and `dv_cur` are supersets of it (a DV only grows), so each side's *newly* // deleted rows are `dv \ base`, and their overlap is `(dv_win INTERSECT dv_cur) MINUS base`. - // Empty overlap => the two txns deleted disjoint rows, and `current ; winner` is a valid - // serialization under both WriteSerializable and Serializable (each deleted only rows the - // other did not touch), so the DV union is safe. Non-empty => the same row was deleted - // concurrently: a genuine conflict, left for the standard checks to abort. + // Empty overlap => the two txns deleted disjoint rows, so the DV union is safe as far as the + // writes go (the reads are checked separately, see currentReadsUnaffectedByWinnerDeletes). + // Non-empty => the same row was deleted concurrently: a genuine conflict, left for the + // standard checks to abort. // // N-way: the current txn reconciles against a chain of winners, once per winning commit. // Induction on the number of prior winners `k` already merged into the current txn: @@ -249,26 +374,36 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker newlyDeletedOverlap.and(currentBitmap) newlyDeletedOverlap.andNot(baseBitmap) - if (!newlyDeletedOverlap.isEmpty) { - // Overlapping row-level modification -> genuine conflict, leave for the standard checks. - None + if (newlyDeletedOverlap.isEmpty) { + Some(DisjointFile( + winningAdd, currentAdd, currentRemove.deletionVector, winningBitmap, currentBitmap)) } else { - // Disjoint: merge the deletion vectors and rebase onto the winner's post-image. - val mergedBitmap = winningBitmap.copy() - mergedBitmap.merge(currentBitmap) - val mergedDescriptor = writeMergedDeletionVector(dvStore, tablePath, mergedBitmap) - // Keep the current AddFile's identity (base row ID / default row commit version already - // reconciled by the row-ID phases) but point it at the merged DV. - val rebasedAdd = currentAdd - .copy(deletionVector = mergedDescriptor, dataChange = true) - .withoutTightBoundStats - // Tombstone the winner's now-live AddFile (carries the winning DV) instead of the stale - // pre-image. - val rebasedRemove = winningAdd.removeWithTimestamp() - Some((rebasedAdd, rebasedRemove)) + None } } + /** + * Merges a disjoint shared file's deletion vectors, persists the merged DV and returns the + * rebased `(AddFile, RemoveFile)` to substitute into the current transaction. + */ + private def mergeFileDeletionVectors( + dvStore: DeletionVectorStore, + tablePath: Path, + file: DisjointFile): (AddFile, RemoveFile) = { + val mergedBitmap = file.winningBitmap.copy() + mergedBitmap.merge(file.currentBitmap) + val mergedDescriptor = writeMergedDeletionVector(dvStore, tablePath, mergedBitmap) + // Keep the current AddFile's identity (base row ID / default row commit version already + // reconciled by the row-ID phases) but point it at the merged DV. + val rebasedAdd = file.currentAdd + .copy(deletionVector = mergedDescriptor, dataChange = true) + .withoutTightBoundStats + // Tombstone the winner's now-live AddFile (carries the winning DV) instead of the stale + // pre-image. + val rebasedRemove = file.winningAdd.removeWithTimestamp() + (rebasedAdd, rebasedRemove) + } + /** Reads a deletion vector into a [[RoaringBitmapArray]], returning an empty bitmap for none. */ protected def readDeletionVectorOrEmpty( dvStore: DeletionVectorStore, diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala index edc43929b81..7baa3f5b52b 100644 --- a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -20,6 +20,7 @@ import java.io.File import scala.concurrent.duration.Duration +import org.apache.spark.sql.delta.actions.{Action, AddFile, RemoveFile} import org.apache.spark.sql.delta.concurrency.PhaseLockingTestMixin import org.apache.spark.sql.delta.concurrency.TransactionExecutionTestMixin import org.apache.spark.sql.delta.sources.DeltaSQLConf @@ -27,8 +28,10 @@ import org.apache.spark.sql.delta.test.DeltaSQLCommandTest import org.apache.spark.{SparkConf, SparkException} import org.apache.spark.sql.{QueryTest, Row} +import org.apache.spark.sql.catalyst.expressions.{AttributeReference, EqualTo, Literal} import org.apache.spark.sql.functions.lit import org.apache.spark.sql.test.SharedSparkSession +import org.apache.spark.sql.types.LongType import org.apache.spark.util.ThreadUtils /** @@ -153,8 +156,9 @@ class RowLevelConcurrencySuite extends QueryTest val log = createSingleFileTableWithDVs(dir) sql(s"ALTER TABLE ${tableRef(dir)} SET TBLPROPERTIES " + s"('${DeltaConfigs.ISOLATION_LEVEL.key}' = 'Serializable')") - // The `current ; winner` schedule of two disjoint-row deletes is a valid serialization even - // under Serializable (neither read a row the other wrote), so the DV union still commits. + // Each DELETE reads only the rows matching its own condition, which are the rows it deletes, + // so neither read observes the other's disjoint delete. Committing in `winner ; current` + // order is a valid serialization even under Serializable, so the DV union still commits. val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) @@ -177,7 +181,8 @@ class RowLevelConcurrencySuite extends QueryTest // enters the current txn's readFiles as a genuine read (not merely as a delete target). The // winner's disjoint DELETE removes that same file, which under Serializable would trip the // delete-read check -- unless row-level resolution drops the reconciled path from readFiles. - // This confirms that removal holds for a read-then-modify op under the stricter isolation. + // A MERGE without a NOT MATCHED clause cannot observe the winner deleting a row it did not + // modify, so this reconciles under the stricter isolation. val txnA = sqlTxn(mergeMatched(dir, 10, "DELETE"), rowLevelConcurrency = true) val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) @@ -608,4 +613,123 @@ class RowLevelConcurrencySuite extends QueryTest (20L, "delete", firstDeleteVersion))) } } + + // --------------------------------------------------------------------------- + // Reads of the current transaction that observe the winner's deletes + // --------------------------------------------------------------------------- + + /** + * The file actions of `DELETE FROM t WHERE id = ` against the table at `dir`, produced on a + * copy so they can be committed by a hand-driven transaction on `dir`. The copy's DV files are + * copied back so the actions resolve. + */ + private def deleteActionsFromCopy(dir: File, id: Long): Seq[Action] = { + val copy = new File(dir.getParentFile, dir.getName + "-copy") + org.apache.commons.io.FileUtils.copyDirectory(dir, copy) + sql(s"DELETE FROM ${tableRef(copy)} WHERE id = $id") + val copyLog = DeltaLog.forTable(spark, copy.getCanonicalPath) + copy.listFiles().filter(_.getName.contains("deletion_vector")) + .foreach(f => org.apache.commons.io.FileUtils.copyFileToDirectory(f, dir)) + copyLog.getChanges(copyLog.update().version).next()._2 + .filter(a => a.isInstanceOf[AddFile] || a.isInstanceOf[RemoveFile]) + } + + private val idAttr = AttributeReference("id", LongType)() + + /** + * A hand-driven transaction that runs `read`, then commits `DELETE ... WHERE id = 5`'s actions + * after a concurrent `DELETE ... WHERE id = 20` committed. The two deletes are row-disjoint. + */ + private def deleteFiveAfterRead(dir: File, log: DeltaLog)(read: OptimisticTransaction => Unit) + : scala.util.Try[Long] = { + val loserActions = deleteActionsFromCopy(dir, 5) + withSQLConf(DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_ENABLED.key -> "true") { + val loser = log.startTransaction() + read(loser) + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 20") + scala.util.Try(loser.commit(loserActions, DeltaOperations.ManualUpdate)) + } + } + + private def assertConflict(result: scala.util.Try[Long]): Unit = assert( + result.failed.toOption.exists(e => Seq("ConcurrentAppend", "ConcurrentDeleteRead") + .exists(e.getClass.getName.contains)), + s"Expected a concurrency conflict, got: $result") + + test("a read of a row the winner deleted is not reconciled") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + sql(s"ALTER TABLE ${tableRef(dir)} SET TBLPROPERTIES " + + s"('${DeltaConfigs.ISOLATION_LEVEL.key}' = 'Serializable')") + // The loser read row 20 ("delete 5 if 20 exists"); the winner deleted row 20. Merging the + // DVs would commit a read the winner invalidated. + assertConflict( + deleteFiveAfterRead(dir, log)(_.filterFiles(Seq(EqualTo(idAttr, Literal(20L)))))) + assert(ids(dir) === (0L to 99L).filterNot(_ == 20L)) + } + } + + test("a whole-table read is not reconciled") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + assertConflict(deleteFiveAfterRead(dir, log)(_.readWholeTable())) + assert(ids(dir) === (0L to 99L).filterNot(_ == 20L)) + } + } + + test("a read the winner's deletes do not match is reconciled") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + sql(s"ALTER TABLE ${tableRef(dir)} SET TBLPROPERTIES " + + s"('${DeltaConfigs.ISOLATION_LEVEL.key}' = 'Serializable')") + val result = + deleteFiveAfterRead(dir, log)(_.filterFiles(Seq(EqualTo(idAttr, Literal(30L))))) + assert(result.isSuccess, s"Expected the DVs to merge, got: $result") + assert(ids(dir) === (0L to 99L).filterNot(id => id == 5L || id == 20L)) + assert(deletionVectorCardinalities(log) === Seq(2L)) + } + } + + test("MERGE with a conditional matched clause and an insert is not reconciled") { + withTempDir { dir => + createSingleFileTableWithDVs(dir) + // Source {5, 20}: 5 matches and is deleted; 20 matches the ON condition but not the clause, + // so it is left alone and NOT inserted. After the winner deletes 20, the same MERGE would + // insert 20 -- so committing the MERGE after the winner without rerunning it is wrong. + val txnA = sqlTxn( + s"""MERGE INTO ${tableRef(dir)} t USING (SELECT explode(array(5L, 20L)) AS id) s + |ON t.id = s.id + |WHEN MATCHED AND s.id = 5 THEN DELETE + |WHEN NOT MATCHED THEN INSERT *""".stripMargin, + rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + assert(ids(dir) === (0L to 99L).filterNot(_ == 20L)) + } + } + + test("MERGE with an unconditional matched clause and an insert reconciles") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + // Every matched row is modified, so a winner can only delete rows the MERGE did not match, + // which cannot change what it inserts. + val txnA = sqlTxn( + s"""MERGE INTO ${tableRef(dir)} t USING (SELECT explode(array(5L, 1000L)) AS id) s + |ON t.id = s.id + |WHEN MATCHED THEN DELETE + |WHEN NOT MATCHED THEN INSERT *""".stripMargin, + rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + assert(ids(dir) === ((0L to 99L).filterNot(id => id == 5L || id == 20L) :+ 1000L)) + assert(deletionVectorCardinalities(log).contains(2L)) + } + } } From cb656e0bc75adc6c756287111cc0dcfb7e9115ef Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Tue, 20 Oct 2026 23:47:36 -0700 Subject: [PATCH 13/16] Do not reconcile RESTORE or DVs that lose rows A deletion vector only grows under DELETE/UPDATE/MERGE, and both the overlap test and the read check rely on that. RESTORE can bring deleted rows back (e.g. Remove(P, {1,2}) + Add(P, {1})): the DV union would delete row 2 again, and the signed-weight read check could cancel a newly deleted read row against a restored one. Skip row-level resolution when either the current or the winning transaction is a RESTORE, and only reconcile a shared file when both the winner's and the current DV contain the base DV, so any commit that brings rows back is left to the file-level checks regardless of its operation name. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Eunjin Song --- .../delta/RowLevelConcurrencyResolution.scala | 22 +++- .../sql/delta/RowLevelConcurrencySuite.scala | 115 +++++++++++++++++- 2 files changed, 129 insertions(+), 8 deletions(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala index 7b5c386aea5..5f83cc832ce 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala @@ -108,6 +108,11 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker if (!rowLevelConcurrencyEnabled) return // A whole-table read observes every row, so any row the winner deleted affects it. if (currentTransactionInfo.readWholeTable) return + // RESTORE is not reconciled: it can bring deleted rows back, which the DV merge does not model. + if (currentTransactionInfo.op.name == DeltaOperations.OP_RESTORE || + winningOperationName.contains(DeltaOperations.OP_RESTORE)) { + return + } // Winning transaction's DV updates: path present in both an AddFile (with a DV) and a // RemoveFile of the winning commit. @@ -352,8 +357,14 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker val winningBitmap = bitmapOf(winningAdd.deletionVector) val currentBitmap = bitmapOf(currentAdd.deletionVector) // `baseBitmap` is the DV of `P` at the current txn's read time (carried on its RemoveFile). - // Both `dv_win` and `dv_cur` are supersets of it (a DV only grows), so each side's *newly* - // deleted rows are `dv \ base`, and their overlap is `(dv_win INTERSECT dv_cur) MINUS base`. + // DELETE/UPDATE/MERGE only add rows to a DV, so `dv_win` and `dv_cur` contain `base` and each + // side's *newly* deleted rows are `dv \ base`. A commit that brings deleted rows back breaks + // this: the union would delete those rows again, and the read check would not see them. Such + // a file is not reconciled. + if (!containsAll(winningBitmap, baseBitmap) || !containsAll(currentBitmap, baseBitmap)) { + return None + } + // The newly-deleted rows overlap in `(dv_win INTERSECT dv_cur) MINUS base`. // Empty overlap => the two txns deleted disjoint rows, so the DV union is safe as far as the // writes go (the reads are checked separately, see currentReadsUnaffectedByWinnerDeletes). // Non-empty => the same row was deleted concurrently: a genuine conflict, left for the @@ -382,6 +393,13 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker } } + /** Whether `bitmap` contains every row of `subset`. */ + private def containsAll(bitmap: RoaringBitmapArray, subset: RoaringBitmapArray): Boolean = { + val missing = subset.copy() + missing.andNot(bitmap) + missing.isEmpty + } + /** * Merges a disjoint shared file's deletion vectors, persists the merged DV and returns the * rebased `(AddFile, RemoveFile)` to substitute into the current transaction. diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala index 7baa3f5b52b..3191a5e49be 100644 --- a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -171,7 +171,8 @@ class RowLevelConcurrencySuite extends QueryTest } } - test("Serializable: a MERGE matched-delete (read-then-modify) reconciles vs a concurrent DELETE") { + test("Serializable: a MERGE matched-delete (read-then-modify) reconciles vs a concurrent " + + "DELETE") { withTempDir { dir => val log = createSingleFileTableWithDVs(dir) sql(s"ALTER TABLE ${tableRef(dir)} SET TBLPROPERTIES " + @@ -362,8 +363,8 @@ class RowLevelConcurrencySuite extends QueryTest "conflicts") { withTempDir { dir => val log = createSingleFileTableWithDVs(dir) - // A (loser) deletes id=10; B (winner) matched-updates id=20 -> 1020, which masks row 20 with a - // DV and appends an image file for 1020. Like the standalone-UPDATE case above, that image + // A (loser) deletes id=10; B (winner) matched-updates id=20 -> 1020, which masks row 20 with + // a DV and appends an image file for 1020. Like the standalone-UPDATE case above, that image // file is a non-blind changed-data add the append check cannot prove disjoint, so the loser // conservatively aborts rather than reconciling. val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 10", rowLevelConcurrency = true) @@ -623,10 +624,14 @@ class RowLevelConcurrencySuite extends QueryTest * copy so they can be committed by a hand-driven transaction on `dir`. The copy's DV files are * copied back so the actions resolve. */ - private def deleteActionsFromCopy(dir: File, id: Long): Seq[Action] = { - val copy = new File(dir.getParentFile, dir.getName + "-copy") + private def deleteActionsFromCopy(dir: File, id: Long): Seq[Action] = + fileActionsFromCopy(dir)(copy => s"DELETE FROM ${tableRef(copy)} WHERE id = $id") + + /** The file actions of the `sqlOnCopy(copy)` command, produced on a copy of the table. */ + private def fileActionsFromCopy(dir: File)(sqlOnCopy: File => String): Seq[Action] = { + val copy = new File(dir.getParentFile, s"${dir.getName}-copy-${java.util.UUID.randomUUID()}") org.apache.commons.io.FileUtils.copyDirectory(dir, copy) - sql(s"DELETE FROM ${tableRef(copy)} WHERE id = $id") + sql(sqlOnCopy(copy)) val copyLog = DeltaLog.forTable(spark, copy.getCanonicalPath) copy.listFiles().filter(_.getName.contains("deletion_vector")) .foreach(f => org.apache.commons.io.FileUtils.copyFileToDirectory(f, dir)) @@ -690,6 +695,104 @@ class RowLevelConcurrencySuite extends QueryTest } } + // --------------------------------------------------------------------------- + // RESTORE and other commits that bring deleted rows back + // --------------------------------------------------------------------------- + + private def restoreTo(dir: File, version: Long): String = + s"RESTORE TABLE ${tableRef(dir)} TO VERSION AS OF $version" + + test("RESTORE winner that brings back a deleted row is not reconciled") { + withTempDir { dir => + createSingleFileTableWithDVs(dir) // v0 + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 1") // v1: dv = {1} + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 2") // v2: dv = {1, 2} + // A prepares dv = {1, 2, 50}; B restores dv = {1}. Merging them would delete row 2 again. + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 50", rowLevelConcurrency = true) + val txnB = sqlTxn(restoreTo(dir, 1), rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + assert(ids(dir) === (0L to 99L).filterNot(_ == 1L)) + } + } + + test("RESTORE winner that only deletes rows is not reconciled") { + withTempDir { dir => + createSingleFileTableWithDVs(dir) // v0 + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 1") // v1: dv = {1} + sql(restoreTo(dir, 0)) // v2: no dv + // B restores dv = {1}, a superset of A's base. RESTORE is still left to the file checks. + val txnA = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 50", rowLevelConcurrency = true) + val txnB = sqlTxn(restoreTo(dir, 1), rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + assert(ids(dir) === (0L to 99L).filterNot(_ == 1L)) + } + } + + test("RESTORE loser aborts on a concurrent DELETE") { + withTempDir { dir => + createSingleFileTableWithDVs(dir) // v0 + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 1") // v1: dv = {1} + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 2") // v2: dv = {1, 2} + // A restores dv = {1}; B deletes row 50 from the same file. RESTORE commits through + // `commitLarge`, which aborts on any concurrent commit without running conflict resolution. + val txnA = sqlTxn(restoreTo(dir, 1), rowLevelConcurrency = true) + val txnB = sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 50", rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assert(e.getCause.isInstanceOf[io.delta.exceptions.ConcurrentWriteException], e.getCause) + assert(ids(dir) === (0L to 99L).filterNot(id => Set(1L, 2L, 50L).contains(id))) + } + } + + test("RESTORE loser committed through conflict resolution is not reconciled") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) // v0 + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 1") // v1: dv = {1} + sql(restoreTo(dir, 0)) // v2: no dv + // The loser restores dv = {1} (rows only added) after reading row 1; the winner deletes row + // 20. Both DV checks pass, so only the RESTORE exclusion keeps this from reconciling. + val loserActions = fileActionsFromCopy(dir)(restoreTo(_, 1)) + val result = withSQLConf(DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_ENABLED.key -> "true") { + val loser = log.startTransaction() + loser.filterFiles(Seq(EqualTo(idAttr, Literal(1L)))) + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 20") + scala.util.Try(loser.commit(loserActions, DeltaOperations.Restore(Some(1L), None))) + } + assertConflict(result) + assert(ids(dir) === (0L to 99L).filterNot(_ == 20L)) + } + } + + test("a non-RESTORE commit that brings back a deleted row is not reconciled") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) // v0 + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 1") // v1: dv = {1} + sql(s"DELETE FROM ${tableRef(dir)} WHERE id = 2") // v2: dv = {1, 2} + // The winner un-deletes row 2 like RESTORE does, but under another operation name (e.g. + // written by another engine). The DVs themselves show the winner's DV lost a base row. + val winnerActions = fileActionsFromCopy(dir)(restoreTo(_, 1)) + val loserActions = deleteActionsFromCopy(dir, 50) + val result = withSQLConf(DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_ENABLED.key -> "true") { + val loser = log.startTransaction() + loser.filterFiles(Seq(EqualTo(idAttr, Literal(50L)))) + log.startTransaction().commit(winnerActions, DeltaOperations.ManualUpdate) + scala.util.Try(loser.commit(loserActions, DeltaOperations.ManualUpdate)) + } + assertConflict(result) + assert(ids(dir) === (0L to 99L).filterNot(_ == 1L)) + } + } + test("MERGE with a conditional matched clause and an insert is not reconciled") { withTempDir { dir => createSingleFileTableWithDVs(dir) From 5813b9e9bf6056b936e92c394154acde92c534ba Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Tue, 20 Oct 2026 23:47:42 -0700 Subject: [PATCH 14/16] Apply the MERGE read shortcut only when the target scan is the only read The shortcut proves a MERGE unaffected by the winner's deletes from its clauses alone, which assumes every target row's fate depends only on that row and the source. A MERGE whose source reads the target table breaks that: two crossed self-MERGEs (A reads row 20 and deletes 5, B reads row 5 and deletes 20) both committed, deleting rows that no serial order deletes together. Source reads of the target are recorded as extra read predicates (a normal MERGE records one, the target scan; the crossed self-MERGE records four), so take the shortcut only when there is exactly one and otherwise fall back to the read-check scan. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Eunjin Song --- .../delta/RowLevelConcurrencyResolution.scala | 12 ++++++---- .../sql/delta/RowLevelConcurrencySuite.scala | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala index 5f83cc832ce..072794aa7c6 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala @@ -244,16 +244,20 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker * subquery-free conjuncts (dropping a conjunct only matches more rows), a read with none of * those matches every row, and any failure counts as affected. * - * A MERGE's target read usually matches every row, but most MERGEs are provably unaffected - * without a scan, see [[mergeUnaffectedByDeletesOfUntouchedRows]]. + * A MERGE's target read usually matches every row, but most MERGEs whose only read is that + * target scan are provably unaffected without a scan, see + * [[mergeUnaffectedByDeletesOfUntouchedRows]]. */ private def currentReadsUnaffectedByWinnerDeletes(files: Seq[DisjointFile]): Boolean = { + val readPredicates = currentTransactionInfo.readPredicates currentTransactionInfo.op match { - case merge: DeltaOperations.Merge if mergeUnaffectedByDeletesOfUntouchedRows(merge) => + // The shortcut reasons about the target scan only, so it applies only when that scan is the + // transaction's only read: a MERGE whose source also reads the table records more reads. + case merge: DeltaOperations.Merge + if readPredicates.size == 1 && mergeUnaffectedByDeletesOfUntouchedRows(merge) => return true case _ => } - val readPredicates = currentTransactionInfo.readPredicates if (readPredicates.isEmpty) return false val conjunctsPerRead = readPredicates.map { rp => (rp.partitionPredicates ++ rp.dataPredicates).filter { e => diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala index 3191a5e49be..d31cc6adec5 100644 --- a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -359,6 +359,28 @@ class RowLevelConcurrencySuite extends QueryTest } } + test("crossed MERGEs whose sources read the target table are not reconciled") { + withTempDir { dir => + createSingleFileTableWithDVs(dir) + // A reads row 20 as its source and deletes row 5; B reads row 5 and deletes row 20. The + // deletes are disjoint, but either serial order deletes only one row: the second MERGE's + // source is empty. A's source read of row 20 must abort it. + def selfMerge(sourceId: Long, targetId: Long): String = + s"""MERGE INTO ${tableRef(dir)} t + |USING (SELECT id + ${targetId - sourceId} AS id FROM ${tableRef(dir)} + | WHERE id = $sourceId) s + |ON t.id = s.id WHEN MATCHED THEN DELETE""".stripMargin + val txnA = sqlTxn(selfMerge(sourceId = 20, targetId = 5), rowLevelConcurrency = true) + val txnB = sqlTxn(selfMerge(sourceId = 5, targetId = 20), rowLevelConcurrency = true) + + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureB, Duration.Inf) + val e = intercept[SparkException] { ThreadUtils.awaitResult(futureA, Duration.Inf) } + assertConcurrentModificationException(e) + assert(ids(dir) === (0L to 99L).filterNot(_ == 20L)) + } + } + test("DELETE (loser) vs MERGE matched-update (winner): winner image file conservatively " + "conflicts") { withTempDir { dir => From 23e1b379ac1687fbe5c3ac19bfac16d5f7cb08b8 Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Tue, 20 Oct 2026 23:53:33 -0700 Subject: [PATCH 15/16] Skip the read-check scan for a DELETE/UPDATE whose only read is its condition A DELETE or UPDATE marks every row of the shared file that matches its condition, so when that condition is its only read the disjointness test already shows that none of the winner's newly deleted rows match it. Return early instead of scanning both images of every shared file, which could reread the predicate columns of large files for single-row DMLs. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Eunjin Song --- .../delta/RowLevelConcurrencyResolution.scala | 14 +++-- .../sql/delta/RowLevelConcurrencySuite.scala | 63 ++++++++++++++++++- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala index 072794aa7c6..4f754586be2 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala @@ -234,7 +234,8 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker * winner's deletes. * * The winner's newly-deleted rows of a file are its live rows under the current transaction's - * base DV minus its live rows under the winner's DV (the latter is a subset), so for the read + * base DV minus its live rows under the winner's DV (a subset, as the winner's DV contains the + * base DV, see [[disjointFileDeletionVectors]]), so for the read * condition `cond` (OR across reads, AND within a read): * * matches(newly deleted) = matches(base-DV live rows) - matches(winner-DV live rows) @@ -244,13 +245,18 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker * subquery-free conjuncts (dropping a conjunct only matches more rows), a read with none of * those matches every row, and any failure counts as affected. * - * A MERGE's target read usually matches every row, but most MERGEs whose only read is that - * target scan are provably unaffected without a scan, see - * [[mergeUnaffectedByDeletesOfUntouchedRows]]. + * Two cases are provably unaffected without a scan: a DELETE/UPDATE whose only read is its own + * condition, and most MERGEs whose only read is the target scan (which usually matches every + * row), see [[mergeUnaffectedByDeletesOfUntouchedRows]]. */ private def currentReadsUnaffectedByWinnerDeletes(files: Seq[DisjointFile]): Boolean = { val readPredicates = currentTransactionInfo.readPredicates currentTransactionInfo.op match { + // A DELETE/UPDATE whose only read is its own condition marked every row of the shared files + // that matches it, so the disjointness test already shows none of the winner's new deletes + // match. No scan needed. + case _: DeltaOperations.Delete | _: DeltaOperations.Update if readPredicates.size == 1 => + return true // The shortcut reasons about the target scan only, so it applies only when that scan is the // transaction's only read: a MERGE whose source also reads the table records more reads. case merge: DeltaOperations.Merge diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala index d31cc6adec5..3492636a2db 100644 --- a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -17,21 +17,26 @@ package org.apache.spark.sql.delta import java.io.File +import java.util.concurrent.atomic.AtomicInteger import scala.concurrent.duration.Duration import org.apache.spark.sql.delta.actions.{Action, AddFile, RemoveFile} import org.apache.spark.sql.delta.concurrency.PhaseLockingTestMixin import org.apache.spark.sql.delta.concurrency.TransactionExecutionTestMixin +import org.apache.spark.sql.delta.files.TahoeBatchFileIndex import org.apache.spark.sql.delta.sources.DeltaSQLConf import org.apache.spark.sql.delta.test.DeltaSQLCommandTest import org.apache.spark.{SparkConf, SparkException} import org.apache.spark.sql.{QueryTest, Row} import org.apache.spark.sql.catalyst.expressions.{AttributeReference, EqualTo, Literal} +import org.apache.spark.sql.execution.QueryExecution +import org.apache.spark.sql.execution.datasources.{HadoopFsRelation, LogicalRelation} import org.apache.spark.sql.functions.lit import org.apache.spark.sql.test.SharedSparkSession import org.apache.spark.sql.types.LongType +import org.apache.spark.sql.util.QueryExecutionListener import org.apache.spark.util.ThreadUtils /** @@ -104,6 +109,35 @@ class RowLevelConcurrencySuite extends QueryTest s"Expected a concurrency conflict, got: $causeName") } + /** Runs `f` and returns how many row-level read-check scans it issued. */ + private def countReadCheckScans(f: => Unit): Int = { + val count = new AtomicInteger(0) + def isReadCheck(qe: QueryExecution): Boolean = qe.analyzed.exists { + case l: LogicalRelation => l.relation match { + case h: HadoopFsRelation => h.location match { + case index: TahoeBatchFileIndex => index.actionType == "rowLevelConcurrencyReadCheck" + case _ => false + } + case _ => false + } + case _ => false + } + val listener = new QueryExecutionListener { + override def onSuccess(funcName: String, qe: QueryExecution, durationNs: Long): Unit = + if (isReadCheck(qe)) count.incrementAndGet() + override def onFailure(funcName: String, qe: QueryExecution, exception: Exception): Unit = + if (isReadCheck(qe)) count.incrementAndGet() + } + spark.listenerManager.register(listener) + try { + f + spark.sparkContext.listenerBus.waitUntilEmpty() + } finally { + spark.listenerManager.unregister(listener) + } + count.get() + } + private def ids(dir: File): Seq[Long] = spark.read.format("delta").load(dir.getAbsolutePath).select("id") .collect().map(_.getLong(0)).sorted.toSeq @@ -696,6 +730,27 @@ class RowLevelConcurrencySuite extends QueryTest } } + for ((name, loserSql) <- Seq( + "DELETE" -> ((dir: File) => s"DELETE FROM ${tableRef(dir)} WHERE id = 50"), + "UPDATE" -> ((dir: File) => s"UPDATE ${tableRef(dir)} SET id = 1050 WHERE id = 50"))) { + test(s"a $name whose only read is its condition reconciles without a read-check scan") { + withTempDir { dir => + createSingleFileTableWithDVs(dir) + val txnA = sqlTxn(loserSql(dir), rowLevelConcurrency = true) + val txnB = + sqlTxn(s"DELETE FROM ${tableRef(dir)} WHERE id = 20", rowLevelConcurrency = true) + val scans = countReadCheckScans { + val (futureA, futureB) = runTxnsWithOrder__A_Start__B__A_End(txnA, txnB) + ThreadUtils.awaitResult(futureA, Duration.Inf) + ThreadUtils.awaitResult(futureB, Duration.Inf) + } + assert(scans === 0) + val expected = (0L to 99L).filterNot(id => id == 20L || id == 50L) + assert(ids(dir) === (if (name == "UPDATE") (expected :+ 1050L).sorted else expected)) + } + } + } + test("a whole-table read is not reconciled") { withTempDir { dir => val log = createSingleFileTableWithDVs(dir) @@ -709,9 +764,13 @@ class RowLevelConcurrencySuite extends QueryTest val log = createSingleFileTableWithDVs(dir) sql(s"ALTER TABLE ${tableRef(dir)} SET TBLPROPERTIES " + s"('${DeltaConfigs.ISOLATION_LEVEL.key}' = 'Serializable')") - val result = - deleteFiveAfterRead(dir, log)(_.filterFiles(Seq(EqualTo(idAttr, Literal(30L))))) + var result: scala.util.Try[Long] = null + val scans = countReadCheckScans { + result = deleteFiveAfterRead(dir, log)(_.filterFiles(Seq(EqualTo(idAttr, Literal(30L))))) + } assert(result.isSuccess, s"Expected the DVs to merge, got: $result") + // A read that is not the DML's own condition is checked by scanning the shared file. + assert(scans === 1) assert(ids(dir) === (0L to 99L).filterNot(id => id == 5L || id == 20L)) assert(deletionVectorCardinalities(log) === Seq(2L)) } From 9a25df818b0656fda57f2d543e1f1f378ddb98c9 Mon Sep 17 00:00:00 2001 From: Eunjin Song Date: Wed, 21 Oct 2026 00:03:51 -0700 Subject: [PATCH 16/16] Resolve row-level conflicts after the file-level checks, with packed DVs Row-level resolution used to decode deletion vectors, scan reads and write a merged DV file per shared file before the remaining file-level checks ran, so a transaction that then aborted (e.g. on a winner's UPDATE image file) had done that work for nothing and left the merged DV files orphaned. It also held every shared file's bitmaps in driver memory at once, and wrote one DV file per file. Split it in two. `prepareRowLevelConflictResolution` only compares actions: it sets aside the files whose DV both transactions updated and drops them from the winning summary and from the current read files. The file-level checks then run against everything else. `resolveRowLevelConflicts` runs last: it decodes the DVs, runs the read check and writes the merged DVs, and throws a ConcurrentDeleteDelete/DeleteRead exception if any file cannot be reconciled. Merged DVs are written through one PackedDeletionVectorWriter, packed up to deletionVectors.packing.targetSize like the DML write path. Decoded bitmaps are kept between the check and the merge only up to the new internal conf rowLevelConcurrency.maxRetainedDeletionVectorBytes (64MB); the rest are read again when merged, in bounded batches. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Eunjin Song --- .../spark/sql/delta/ConflictChecker.scala | 22 +- .../delta/RowLevelConcurrencyResolution.scala | 363 +++++++++++------- .../sql/delta/sources/DeltaSQLConf.scala | 12 + .../sql/delta/RowLevelConcurrencySuite.scala | 46 ++- 4 files changed, 285 insertions(+), 158 deletions(-) diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala index c09b6743205..72e3a8b790a 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/ConflictChecker.scala @@ -272,10 +272,10 @@ object WinningCommitSummary { private[delta] class ConflictChecker( protected val spark: SparkSession, initialCurrentTransactionInfo: CurrentTransactionInfo, - // A `var` (reassigned once by [[resolveRowLevelConflicts]]) to drop files that were reconciled - // at the row level, mirroring how `currentTransactionInfo` is rebased. After that the - // file-level checks run against a summary that never mentions the reconciled files, so they - // need no changes. + // A `var` (reassigned once by [[prepareRowLevelConflictResolution]]) to drop files that are + // resolved at the row level, mirroring how `currentTransactionInfo` is rebased. After that the + // file-level checks run against a summary that never mentions those files, so they need no + // changes. protected var winningCommitSummary: WinningCommitSummary, isolationLevel: IsolationLevel) extends DeltaLogging with ConflictCheckerPredicateElimination @@ -357,11 +357,11 @@ private[delta] class ConflictChecker( // Update the table version in newly added type widening metadata. updateTypeWideningMetadata() - // Row-level concurrency: try to resolve "same physical file" conflicts by merging deletion - // vectors before the file-level checks run, so that concurrent DML touching disjoint rows of - // the same file no longer aborts. Runs after row-ID reassignment so merged files keep stable - // base row IDs. - resolveRowLevelConflicts() + // Row-level concurrency: set aside the "same physical file" conflicts that merging deletion + // vectors may resolve, so that concurrent DML touching disjoint rows of the same file no + // longer aborts. The file-level checks below then see only the other conflicts. Runs after + // row-ID reassignment so merged files keep stable base row IDs. + prepareRowLevelConflictResolution() // Data file checks. checkForAddedFilesThatShouldHaveBeenReadByCurrentTxn() @@ -369,6 +369,10 @@ private[delta] class ConflictChecker( checkForDeletedFilesAgainstCurrentTxnDeletedFiles() resolveTimestampOrderingConflicts() + // Resolve the set-aside files last, so a transaction that aborts on another conflict decodes + // no deletion vector, scans no read and writes no merged deletion vector. + resolveRowLevelConflicts() + recordRowLevelConflictResolution() // Fold the winning commit into our transaction info before the next attempt: this advances diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala index 4f754586be2..52a57d7973b 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/RowLevelConcurrencyResolution.scala @@ -16,14 +16,13 @@ package org.apache.spark.sql.delta -import java.util.UUID - import scala.collection.mutable import scala.util.control.NonFatal import org.apache.spark.sql.delta.ClassicColumnConversions._ import org.apache.spark.sql.delta.actions.{AddFile, DeletionVectorDescriptor, RemoveFile} -import org.apache.spark.sql.delta.commands.DeletionVectorUtils +import org.apache.spark.sql.delta.commands.{DeletionVectorUtils, DeletionVectorWriter} +import org.apache.spark.sql.delta.commands.DeletionVectorWriter.DeletionVectorMapperContext import org.apache.spark.sql.delta.deletionvectors.{RoaringBitmapArray, RoaringBitmapArrayFormat} import org.apache.spark.sql.delta.logging.DeltaLogKeys import org.apache.spark.sql.delta.metering.DeltaLogging @@ -45,8 +44,8 @@ import org.apache.spark.util.ThreadUtils * Mixed into [[ConflictChecker]] as a self-typed trait: it reads and rewrites the checker's * transaction state (`currentTransactionInfo`, `winningCommitSummary`, `deltaLog`, `spark`) * directly, and lives in its own file so ConflictChecker stays focused on file-level conflict - * detection. The two deletion-vector helpers are `protected` so the OPTIMIZE-vs-DML reconciliation - * (which mixes into the same checker) can reuse them. + * detection. [[readDeletionVectorOrEmpty]] is `protected` and [[PackedDeletionVectorWriter]] is + * shared so the OPTIMIZE-vs-DML reconciliation (which mixes into the same checker) can reuse them. */ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker => @@ -61,7 +60,9 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker winningCommitSummary.commitInfo.map(_.operation) /** - * Resolves "same physical file" conflicts with the winning transaction at the row level. + * Resolves "same physical file" conflicts with the winning transaction at the row level, for the + * files set aside by [[prepareRowLevelConflictResolution]]. Runs once every other check against + * the winning commit passed, and throws the conflict when any of the files cannot be reconciled. * * A DV-based DELETE/UPDATE emits, for each touched file `P`, a `RemoveFile(P)` (tombstone of the * pre-image) and an `AddFile(P)` carrying a larger deletion vector. When both the winning and the @@ -70,12 +71,12 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker * 1. decode the winning DV, the current DV and their common base DV (from the pre-image * `RemoveFile`) as [[RoaringBitmapArray]]s; * 2. check whether the newly-deleted rows are disjoint, i.e. `(dv_win INTERSECT dv_cur) MINUS - * dv_base` is empty. If they overlap, this is a genuine row-level conflict and we leave the - * file for the standard checks to abort; - * 3. on disjoint sets, merge the DVs (`dv_win UNION dv_cur`), persist a new DV file, - * and rebase the current transaction onto the winner's post-image: the current `AddFile(P)` - * now carries the merged DV and the current `RemoveFile(P)` now tombstones the winner's - * `AddFile(P)`. + * dv_base` is empty. If they overlap, this is a genuine row-level conflict and the commit + * aborts; + * 3. on disjoint sets, merge the DVs (`dv_win UNION dv_cur`), persist them packed into new DV + * files, and rebase the current transaction onto the winner's post-image: the current + * `AddFile(P)` now carries the merged DV and the current `RemoveFile(P)` now tombstones the + * winner's `AddFile(P)`. * * Worked example: file `P` holds rows 0..99, undeleted at the current txn's read time * (`dv_base = {}`). The winner commits `DELETE WHERE id IN (5, 10)` so `dv_win = {5, 10}`; the @@ -83,8 +84,8 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker * `({5,10} INTERSECT {20,21}) MINUS {} = {}` -> disjoint, so we merge to `dv = {5, 10, 20, 21}`, * point the current `AddFile(P)` at it, and tombstone the winner's `AddFile(P)`; both deletes * survive. Had the current txn instead deleted row 5 (`dv_cur = {5, 21}`), the overlap would be - * `{5}` -> the same row was deleted twice concurrently, a genuine conflict left for the standard - * checks to abort. (For a 3+ way chain `dv_base` is the previous winner's DV rather than the + * `{5}` -> the same row was deleted twice concurrently, a genuine conflict that aborts the + * commit. (For a 3+ way chain `dv_base` is the previous winner's DV rather than the * original pre-image, which keeps the overlap test conservative and the merge correct.) * * Reconciling `P` hides the winner's deletes in `P` from the delete-read check, so a file is only @@ -98,13 +99,52 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker * `MERGE ... WHEN MATCHED AND cond THEN DELETE WHEN NOT MATCHED THEN INSERT` that matched a row * without deleting it must INSERT the source row once the winner deletes that target row. * - * Resolved paths are recorded in [[rowLevelResolvedPaths]] and skipped by the file-level delete - * and append checks. Row identity is preserved for free: the merged file is the same physical - * file, so its base row ID is unchanged and [[reassignOverlappingRowIds]] (already run) leaves it - * alone. Deletion vectors index physical row positions within one immutable Parquet file, so the - * merge needs no row tracking. + * Row identity is preserved for free: the merged file is the same physical file, so its base row + * ID is unchanged and [[reassignOverlappingRowIds]] (already run) leaves it alone. Deletion + * vectors index physical row positions within one immutable Parquet file, so the merge needs no + * row tracking. */ protected def resolveRowLevelConflicts(): Unit = { + if (rowLevelCandidates.isEmpty) return + recordTime("resolved-row-level-conflicts") { + val dvStore = DeletionVectorStore.createInstance(deltaLog.newDeltaHadoopConf()) + val tablePath = deltaLog.dataPath + val replacements = failSafe { + val disjointFiles = checkDisjointDeletionVectors(dvStore, tablePath) + val disjointPaths = disjointFiles.map(_.file.path).toSet + // The two transactions deleted the same row of a file, or one brought deleted rows back. + rowLevelCandidates.find(f => !disjointPaths.contains(f.path)).foreach { f => + throw conflictOnFile(f, deleteRead = false) + } + if (!currentReadsUnaffectedByWinnerDeletes(rowLevelCandidates)) { + throw conflictOnFile(rowLevelCandidates.head, deleteRead = true) + } + writeMergedDeletionVectors(dvStore, tablePath, disjointFiles) + } + + // Rebase the current transaction onto the winner's post-image: each AddFile(P) now carries + // the merged DV and each RemoveFile(P) tombstones the winner's AddFile(P). The winning + // summary and the current read files no longer mention P since the preparation. + val newActions = currentTransactionInfo.actions.map { + case a: AddFile if replacements.contains(a.path) => replacements(a.path)._1 + case r: RemoveFile if replacements.contains(r.path) => replacements(r.path)._2 + case other => other + } + currentTransactionInfo = currentTransactionInfo.copy(actions = newActions) + numRowLevelResolvedPaths = replacements.size + } + } + + /** + * Sets aside the files [[resolveRowLevelConflicts]] may reconcile: those whose deletion vector + * both the winning and the current transaction updated, i.e. a `RemoveFile(P)` plus an + * `AddFile(P)` with a DV on both sides. Only compares actions. + * + * The files are dropped from the winning summary and from the current transaction's read files, + * so the file-level checks that run next see only the other conflicts, and abort before any + * deletion vector is decoded, any read is scanned or any merged deletion vector is written. + */ + protected def prepareRowLevelConflictResolution(): Unit = { if (!rowLevelConcurrencyEnabled) return // A whole-table read observes every row, so any row the winner deleted affects it. if (currentTransactionInfo.readWholeTable) return @@ -138,76 +178,61 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker .intersect(currentRemoveByPath.keySet) if (sharedPaths.isEmpty) return - recordTime("resolved-row-level-conflicts") { - val dvStore = DeletionVectorStore.createInstance(deltaLog.newDeltaHadoopConf()) - val tablePath = deltaLog.dataPath + rowLevelCandidates = sharedPaths.toSeq.map { path => + SharedFile(winningDvUpdates(path), currentAddByPath(path), currentRemoveByPath(path)) + } + currentTransactionInfo = currentTransactionInfo.copy( + readFiles = currentTransactionInfo.readFiles.filterNot(f => sharedPaths.contains(f.path))) + winningCommitSummary = pruneReconciledFiles(winningCommitSummary, sharedPaths) + } - // Each shared file's DVs are decoded and checked for disjointness, and later merged and - // written, independently; the work is driver-side object-store DV I/O. When many files - // conflict, parallelize across a bounded pool to shorten the conflict window; a single file - // stays on the caller thread. - def forEachPath[A, B](items: Seq[A], name: String)(f: A => Option[B]): Seq[B] = { - // Bounded driver parallelism (cf. DeltaFileOperations footer reads, which use 8). - val parallelism = math.min(items.size, 8) - if (parallelism <= 1) items.flatMap(f) - else ThreadUtils.parmap(items, name, parallelism)(f).flatten - } - // Fail safe: DV decode/merge/write is a pure optimization over the conservative default. If - // it fails for a file (unreadable/corrupt DV, transient I/O), skip row-level resolution for - // it so the standard file-level checks abort cleanly with a retryable Concurrent* exception - // instead of surfacing an unexpected error out of conflict detection. - def failSafe[B](path: String)(f: => Option[B]): Option[B] = - try f catch { - case NonFatal(e) => - logWarning(log"Row-level concurrency resolution failed for file " + - log"${MDC(DeltaLogKeys.PATH, path)}; leaving it for the standard conflict checks", e) - None - } + /** The files set aside by [[prepareRowLevelConflictResolution]]. */ + private var rowLevelCandidates: Seq[SharedFile] = Seq.empty - val disjointFiles = forEachPath(sharedPaths.toSeq, "rowLevelConflictCheck") { path => - failSafe(path) { - disjointFileDeletionVectors( - dvStore, tablePath, - winningDvUpdates(path), currentAddByPath(path), currentRemoveByPath(path)) - } - } - // A file the winner and current txn both deleted rows from cannot be reconciled, and leaves a - // conflict for the standard checks anyway. Likewise if the winner's deletes are visible to - // the current txn's reads. Either way nothing is merged, so no DV file is written. - val reconcilable = disjointFiles.size == sharedPaths.size && - currentReadsUnaffectedByWinnerDeletes(disjointFiles) - val reconciled = if (!reconcilable) Seq.empty else { - forEachPath(disjointFiles, "rowLevelConflictResolution") { file => - failSafe(file.currentAdd.path) { - Some(file.currentAdd.path -> mergeFileDeletionVectors(dvStore, tablePath, file)) - } - } - } + /** A file whose deletion vector both the winning and the current transaction updated. */ + private case class SharedFile( + winningAdd: AddFile, + currentAdd: AddFile, + currentRemove: RemoveFile) { + def path: String = currentAdd.path + } - // Apply per-file results on the caller thread (no shared-state races across the pool). - val replacements = reconciled.toMap - if (replacements.nonEmpty) { - val resolvedPaths = replacements.keySet - - // Rewrite BOTH sides of the conflict so the residual is a plain no-conflict state that the - // file-level checks (unchanged from upstream) handle. Current txn: rebase each AddFile(P) - // onto the merged DV and each RemoveFile(P) onto the winner's post-image, and drop P from - // readFiles (it is no longer "read" for the delete-read check). - val newActions = currentTransactionInfo.actions.map { - case a: AddFile if replacements.contains(a.path) => replacements(a.path)._1 - case r: RemoveFile if replacements.contains(r.path) => replacements(r.path)._2 - case other => other - } - val newReadFiles = - currentTransactionInfo.readFiles.filterNot(f => resolvedPaths.contains(f.path)) - currentTransactionInfo = - currentTransactionInfo.copy(actions = newActions, readFiles = newReadFiles) - // Winning side: drop the reconciled AddFile(P)/RemoveFile(P) pair from the summary, so no - // check sees P as a winner-side add or remove. - winningCommitSummary = pruneReconciledFiles(winningCommitSummary, resolvedPaths) - numRowLevelResolvedPaths = resolvedPaths.size - } + /** + * Fail safe: an unreadable or corrupt DV, or a transient I/O error, aborts the commit with a + * retryable conflict instead of surfacing an unexpected error out of conflict detection. + */ + private def failSafe[A](f: => A): A = + try f catch { + case e: DeltaConcurrentModificationException => throw e + case NonFatal(e) => + val file = rowLevelCandidates.head + logWarning(log"Row-level concurrency resolution failed for file " + + log"${MDC(DeltaLogKeys.PATH, file.path)}; aborting with a conflict", e) + throw conflictOnFile(file, deleteRead = false) } + + /** The conflict for a set-aside file that cannot be reconciled. */ + private def conflictOnFile(file: SharedFile, deleteRead: Boolean): Throwable = { + val partitionOpt = getPrettyPartitionMessage(file.currentRemove.partitionValues) + if (deleteRead) { + DeltaErrors.concurrentDeleteReadException( + winningCommitSummary.commitInfo, getTableNameOrPath, winningCommitVersion, partitionOpt) + } else { + DeltaErrors.concurrentDeleteDeleteException( + winningCommitSummary.commitInfo, getTableNameOrPath, winningCommitVersion, partitionOpt) + } + } + + /** + * Applies `f` to each item, across a bounded pool when there are several: the work is + * driver-side object-store DV I/O, so parallelism shortens the conflict window. A single item + * stays on the caller thread. + */ + private def forEachFile[A, B](items: Seq[A], name: String)(f: A => Option[B]): Seq[B] = { + // Bounded driver parallelism (cf. DeltaFileOperations footer reads, which use 8). + val parallelism = math.min(items.size, 8) + if (parallelism <= 1) items.flatMap(f) + else ThreadUtils.parmap(items, name, parallelism)(f).flatten } /** Number of files reconciled by [[resolveRowLevelConflicts]] against this winning commit. */ @@ -249,7 +274,7 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker * condition, and most MERGEs whose only read is the target scan (which usually matches every * row), see [[mergeUnaffectedByDeletesOfUntouchedRows]]. */ - private def currentReadsUnaffectedByWinnerDeletes(files: Seq[DisjointFile]): Boolean = { + private def currentReadsUnaffectedByWinnerDeletes(files: Seq[SharedFile]): Boolean = { val readPredicates = currentTransactionInfo.readPredicates currentTransactionInfo.op match { // A DELETE/UPDATE whose only read is its own condition marked every row of the shared files @@ -281,7 +306,8 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker .reduce(_ || _) df.where(condition).select(lit(weight).as("weight")) } - val baseImages = files.map(f => f.currentAdd.copy(deletionVector = f.baseDeletionVector)) + val baseImages = + files.map(f => f.currentAdd.copy(deletionVector = f.currentRemove.deletionVector)) val winnerImages = files.map(_.winningAdd) val newlyDeletedMatches = matchingRowWeights(baseImages, 1L) .union(matchingRowWeights(winnerImages, -1L)) @@ -340,32 +366,50 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker } /** - * A shared file `P` whose newly-deleted rows are disjoint between the winning and current - * transactions, with the decoded DVs needed to merge them. + * A set-aside file whose newly-deleted rows are disjoint between the winning and current + * transactions, with its decoded `(winning, current)` bitmaps when they are kept for the merge. */ private case class DisjointFile( - winningAdd: AddFile, - currentAdd: AddFile, - baseDeletionVector: DeletionVectorDescriptor, - winningBitmap: RoaringBitmapArray, - currentBitmap: RoaringBitmapArray) + file: SharedFile, + bitmaps: Option[(RoaringBitmapArray, RoaringBitmapArray)]) /** - * Decodes the winning, current and base deletion vectors of a single shared file `P` and returns - * them when the two transactions deleted disjoint rows, or `None` when they overlap (a genuine - * row-level conflict, left for the standard checks). + * Decodes the deletion vectors of every set-aside file and returns those whose newly-deleted + * rows are disjoint. The decoded bitmaps are kept for the merge only while their total size stays + * within [[DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_MAX_RETAINED_DV_BYTES]], so the driver does + * not hold every file's bitmaps at once; the other files' are read again when merged. + */ + private def checkDisjointDeletionVectors( + dvStore: DeletionVectorStore, + tablePath: Path): Seq[DisjointFile] = { + val maxRetainedBytes = + spark.conf.get(DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_MAX_RETAINED_DV_BYTES) + var retainedBytes = 0L + val filesToCheck = rowLevelCandidates.map { f => + retainedBytes += + f.winningAdd.deletionVector.sizeInBytes + f.currentAdd.deletionVector.sizeInBytes + f -> (retainedBytes <= maxRetainedBytes) + } + forEachFile(filesToCheck, "rowLevelConflictCheck") { case (f, retain) => + disjointFileDeletionVectors(dvStore, tablePath, f, retain) + } + } + + /** + * Decodes the winning, current and base deletion vectors of a single set-aside file `P` and + * returns it when the two transactions deleted disjoint rows, or `None` when they cannot be + * merged (a genuine row-level conflict). */ private def disjointFileDeletionVectors( dvStore: DeletionVectorStore, tablePath: Path, - winningAdd: AddFile, - currentAdd: AddFile, - currentRemove: RemoveFile): Option[DisjointFile] = { + file: SharedFile, + retainBitmaps: Boolean): Option[DisjointFile] = { def bitmapOf(dv: DeletionVectorDescriptor): RoaringBitmapArray = readDeletionVectorOrEmpty(dvStore, dv, tablePath) - val baseBitmap = bitmapOf(currentRemove.deletionVector) - val winningBitmap = bitmapOf(winningAdd.deletionVector) - val currentBitmap = bitmapOf(currentAdd.deletionVector) + val baseBitmap = bitmapOf(file.currentRemove.deletionVector) + val winningBitmap = bitmapOf(file.winningAdd.deletionVector) + val currentBitmap = bitmapOf(file.currentAdd.deletionVector) // `baseBitmap` is the DV of `P` at the current txn's read time (carried on its RemoveFile). // DELETE/UPDATE/MERGE only add rows to a DV, so `dv_win` and `dv_cur` contain `base` and each // side's *newly* deleted rows are `dv \ base`. A commit that brings deleted rows back breaks @@ -377,8 +421,8 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker // The newly-deleted rows overlap in `(dv_win INTERSECT dv_cur) MINUS base`. // Empty overlap => the two txns deleted disjoint rows, so the DV union is safe as far as the // writes go (the reads are checked separately, see currentReadsUnaffectedByWinnerDeletes). - // Non-empty => the same row was deleted concurrently: a genuine conflict, left for the - // standard checks to abort. + // Non-empty => the same row was deleted concurrently: a genuine conflict, and the commit + // aborts. // // N-way: the current txn reconciles against a chain of winners, once per winning commit. // Induction on the number of prior winners `k` already merged into the current txn: @@ -396,8 +440,7 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker newlyDeletedOverlap.andNot(baseBitmap) if (newlyDeletedOverlap.isEmpty) { - Some(DisjointFile( - winningAdd, currentAdd, currentRemove.deletionVector, winningBitmap, currentBitmap)) + Some(DisjointFile(file, if (retainBitmaps) Some((winningBitmap, currentBitmap)) else None)) } else { None } @@ -411,25 +454,55 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker } /** - * Merges a disjoint shared file's deletion vectors, persists the merged DV and returns the - * rebased `(AddFile, RemoveFile)` to substitute into the current transaction. + * Merges each disjoint file's deletion vectors and writes them packed into as few new DV files as + * [[DeltaSQLConf.DELETION_VECTOR_PACKING_TARGET_SIZE]] allows, like the DML write path does. + * Bitmaps not kept from the disjointness check are read again here, a bounded batch at a time. + * Returns the current transaction's rebased `(AddFile, RemoveFile)` per path. + * + * NOTE: this writes DV files as a side effect of conflict resolution. If the commit ultimately + * fails or is retried against another winning version, they are orphaned and later reclaimed by + * VACUUM (same lifecycle as any DV written by DML). */ - private def mergeFileDeletionVectors( + private def writeMergedDeletionVectors( dvStore: DeletionVectorStore, tablePath: Path, - file: DisjointFile): (AddFile, RemoveFile) = { - val mergedBitmap = file.winningBitmap.copy() - mergedBitmap.merge(file.currentBitmap) - val mergedDescriptor = writeMergedDeletionVector(dvStore, tablePath, mergedBitmap) + files: Seq[DisjointFile]): Map[String, (AddFile, RemoveFile)] = { + val writer = new PackedDeletionVectorWriter( + dvStore, tablePath, spark.conf.get(DeltaSQLConf.DELETION_VECTOR_PACKING_TARGET_SIZE)) + val replacements = Map.newBuilder[String, (AddFile, RemoveFile)] + try { + // Merge a batch in parallel, then append it to the packed file on this thread. + files.grouped(64).foreach { batch => + val merged = forEachFile(batch, "rowLevelConflictMerge") { f => + val (winningBitmap, currentBitmap) = f.bitmaps.getOrElse(( + readDeletionVectorOrEmpty(dvStore, f.file.winningAdd.deletionVector, tablePath), + readDeletionVectorOrEmpty(dvStore, f.file.currentAdd.deletionVector, tablePath))) + val mergedBitmap = winningBitmap.copy() + mergedBitmap.merge(currentBitmap) + Some(f.file -> mergedBitmap) + } + merged.foreach { case (file, mergedBitmap) => + replacements += file.path -> rebased(file, writer.write(mergedBitmap)) + } + } + } finally { + writer.close() + } + replacements.result() + } + + /** The current transaction's `(AddFile, RemoveFile)` for `file`, rebased onto the winner's. */ + private def rebased( + file: SharedFile, + mergedDeletionVector: DeletionVectorDescriptor): (AddFile, RemoveFile) = { // Keep the current AddFile's identity (base row ID / default row commit version already // reconciled by the row-ID phases) but point it at the merged DV. val rebasedAdd = file.currentAdd - .copy(deletionVector = mergedDescriptor, dataChange = true) + .copy(deletionVector = mergedDeletionVector, dataChange = true) .withoutTightBoundStats // Tombstone the winner's now-live AddFile (carries the winning DV) instead of the stale // pre-image. - val rebasedRemove = file.winningAdd.removeWithTimestamp() - (rebasedAdd, rebasedRemove) + (rebasedAdd, file.winningAdd.removeWithTimestamp()) } /** Reads a deletion vector into a [[RoaringBitmapArray]], returning an empty bitmap for none. */ @@ -439,35 +512,37 @@ trait RowLevelConcurrencyResolution extends DeltaLogging { self: ConflictChecker tablePath: Path): RoaringBitmapArray = { if (dv == null || dv.isEmpty) new RoaringBitmapArray() else dvStore.read(dv, tablePath) } +} - /** - * Persists a merged bitmap to a new deletion vector file and returns its descriptor. - * - * NOTE: this writes a DV file as a side effect of conflict resolution. If the commit ultimately - * fails or is retried against another winning version, the file is orphaned and later reclaimed - * by VACUUM (same lifecycle as any DV written by DML). This mirrors how the DML write path - * persists DVs (see `DeletionVectorWriter.storeSerializedBitmap`). - */ - protected def writeMergedDeletionVector( - dvStore: DeletionVectorStore, - tablePath: Path, - bitmap: RoaringBitmapArray): DeletionVectorDescriptor = { - // An empty DV has no on-disk representation (matches DeletionVectorWriter). +/** + * Writes deletion vectors packed into as few new files as `targetSize` allows, as the DML write + * path does (see [[DeletionVectorWriter.createMapperToStoreDeletionVectors]]). + */ +private[delta] class PackedDeletionVectorWriter( + dvStore: DeletionVectorStore, + tablePath: Path, + targetSize: Long) extends AutoCloseable { + private val tablePathWithFs = dvStore.pathWithFileSystem(tablePath) + private var context: Option[DeletionVectorMapperContext] = None + private var bytesInFile = 0L + + /** Writes `bitmap` and returns its descriptor. An empty bitmap needs no file. */ + def write(bitmap: RoaringBitmapArray): DeletionVectorDescriptor = { if (bitmap.isEmpty) return DeletionVectorDescriptor.EMPTY - val tablePathWithFs = dvStore.pathWithFileSystem(tablePath) - val fileId = UUID.randomUUID() - val writer = dvStore.createWriter(dvStore.generateFileNameInTable(tablePathWithFs, fileId)) - try { - val serialized = DeletionVectorUtils.serialize( - bitmap, RoaringBitmapArrayFormat.Portable, Some(tablePath)) - val range = writer.write(serialized) - DeletionVectorDescriptor.onDiskWithUuidRelativePath( - id = fileId, - sizeInBytes = serialized.length, - cardinality = bitmap.cardinality, - offset = Some(range.offset)) - } finally { - writer.close() + val serialized = + DeletionVectorUtils.serialize(bitmap, RoaringBitmapArrayFormat.Portable, Some(tablePath)) + if (context.isEmpty || bytesInFile + serialized.length > targetSize) { + close() + val (writer, fileId) = DeletionVectorWriter.createWriter(dvStore, tablePathWithFs) + context = Some(DeletionVectorMapperContext(dvStore, writer, tablePath, fileId, prefix = "")) + bytesInFile = 0L } + bytesInFile += serialized.length + DeletionVectorWriter.storeSerializedBitmap(context.get, serialized, bitmap.cardinality) + } + + override def close(): Unit = { + context.foreach(_.writer.close()) + context = None } } diff --git a/spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala b/spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala index 0682c2fa778..e90bf58ad93 100644 --- a/spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala +++ b/spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala @@ -620,6 +620,18 @@ trait DeltaSQLConfBase extends DeltaSQLConfUtils { .booleanConf .createWithDefault(false) + val DELTA_ROW_LEVEL_CONCURRENCY_MAX_RETAINED_DV_BYTES = + buildConf("rowLevelConcurrency.maxRetainedDeletionVectorBytes") + .internal() + .doc( + """Maximum total size of the deletion vectors that row-level concurrency keeps in driver + |memory between checking and merging the files it reconciles. The deletion vectors of + |the remaining files are read again, in bounded batches, when they are merged.""" + .stripMargin) + .bytesConf(ByteUnit.BYTE) + .checkValue(_ >= 0, "rowLevelConcurrency.maxRetainedDeletionVectorBytes must be non-negative") + .createWithDefault(64L * 1024L * 1024L) + val DELTA_PROTOCOL_DEFAULT_WRITER_VERSION = buildConf("properties.defaults.minWriterVersion") .doc("The default writer protocol version to create new tables with, unless a feature " + diff --git a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala index 3492636a2db..db97b0a5a9d 100644 --- a/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala +++ b/spark/src/test/scala/org/apache/spark/sql/delta/RowLevelConcurrencySuite.scala @@ -544,8 +544,14 @@ class RowLevelConcurrencySuite extends QueryTest // parmap preserves input order, so the reconciled result matches the sequential branch. // --------------------------------------------------------------------------- - test("disjoint DELETEs spanning two files reconcile via the bounded-parallel path") { - withTempDir { dir => + for ((name, confs, expectedDvFiles) <- Seq( + ("", Seq.empty[(String, String)], 1), + (", re-reading bitmaps that were not kept", + Seq(DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_MAX_RETAINED_DV_BYTES.key -> "0"), 1), + (", one DV file per packing target size", + Seq(DeltaSQLConf.DELETION_VECTOR_PACKING_TARGET_SIZE.key -> "1"), 2))) + test(s"disjoint DELETEs spanning two files reconcile via the bounded-parallel path$name") { + withTempDir { dir => withSQLConf(confs: _*) { // Two data files: ids [0,100) and [100,200), one file each (separate appends). spark.range(start = 0, end = 100, step = 1, numPartitions = 1) .write.format("delta").mode("append").save(dir.getAbsolutePath) @@ -569,7 +575,10 @@ class RowLevelConcurrencySuite extends QueryTest assert(ids(dir) === (0L to 199L).filterNot(id => Set(10L, 20L, 110L, 120L).contains(id))) // Both files survive, each carrying its own merged DV (winner + loser delete): cardinality 2. assert(deletionVectorCardinalities(log).sorted === Seq(2L, 2L)) - } + // The merged DVs are packed into one DV file unless the packing target size forbids it. + val dvFiles = log.update().allFiles.collect().map(_.deletionVector.pathOrInlineDv).distinct + assert(dvFiles.length === expectedDvFiles) + }} } // --------------------------------------------------------------------------- @@ -713,8 +722,9 @@ class RowLevelConcurrencySuite extends QueryTest } private def assertConflict(result: scala.util.Try[Long]): Unit = assert( - result.failed.toOption.exists(e => Seq("ConcurrentAppend", "ConcurrentDeleteRead") - .exists(e.getClass.getName.contains)), + result.failed.toOption.exists(e => + Seq("ConcurrentAppend", "ConcurrentDeleteRead", "ConcurrentDeleteDelete") + .exists(e.getClass.getName.contains)), s"Expected a concurrency conflict, got: $result") test("a read of a row the winner deleted is not reconciled") { @@ -751,6 +761,32 @@ class RowLevelConcurrencySuite extends QueryTest } } + test("a loser that aborts on another conflict scans no read and writes no merged DV") { + withTempDir { dir => + val log = createSingleFileTableWithDVs(dir) + val loserActions = deleteActionsFromCopy(dir, 5) + def dvFilesOnDisk(): Set[String] = + dir.listFiles().map(_.getName).filter(_.contains("deletion_vector_")).toSet + var result: scala.util.Try[Long] = null + var dvFilesWritten = Set.empty[String] + val scans = countReadCheckScans { + withSQLConf(DeltaSQLConf.DELTA_ROW_LEVEL_CONCURRENCY_ENABLED.key -> "true") { + val loser = log.startTransaction() + loser.filterFiles(Seq(EqualTo(idAttr, Literal(30L)))) + // The winner's UPDATE masks row 20 in the shared file, which alone would reconcile, and + // appends an image file with id 1020, which the loser's read cannot rule out. + sql(s"UPDATE ${tableRef(dir)} SET id = 1020 WHERE id = 20") + val before = dvFilesOnDisk() + result = scala.util.Try(loser.commit(loserActions, DeltaOperations.ManualUpdate)) + dvFilesWritten = dvFilesOnDisk() -- before + } + } + assert(result.failed.toOption.exists(_.isInstanceOf[ConcurrentAppendException]), result) + assert(scans === 0) + assert(dvFilesWritten.isEmpty, dvFilesWritten) + } + } + test("a whole-table read is not reconciled") { withTempDir { dir => val log = createSingleFileTableWithDVs(dir)