Skip to content

Commit 29ef8a5

Browse files
beinanclaude
andcommitted
fix: simplify commitIndexSegments to use Lance core's atomic replacement
Lance core's commitExistingIndexSegments already handles atomic replacement — it finds existing segments with overlapping fragments and removes them in the same CreateIndex transaction. The Spark-side manual cleanup (second dataset open + removal transaction) was redundant and introduced a race condition. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent fbe05fb commit 29ef8a5

1 file changed

Lines changed: 5 additions & 45 deletions

File tree

  • lance-spark-base_2.12/src/main/scala/org/apache/spark/sql/execution/datasources/v2

lance-spark-base_2.12/src/main/scala/org/apache/spark/sql/execution/datasources/v2/AddIndexExec.scala

Lines changed: 5 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -203,57 +203,17 @@ case class AddIndexExec(
203203
UTF8String.fromString(indexName))))
204204
}
205205

206-
// Note: There is a narrow race window between commitExistingIndexSegments and
207-
// the subsequent dataset re-open to remove old segments. A concurrent writer
208-
// could modify the index state in between. Lance's optimistic concurrency
209-
// control will surface a conflict error if this happens.
206+
// Lance core's commitExistingIndexSegments handles atomic replacement:
207+
// it finds existing segments whose fragments overlap with incoming ones
208+
// and removes them in the same CreateIndex transaction.
210209
private def commitIndexSegments(
211210
readOptions: LanceSparkReadOptions,
212211
column: String,
213212
segments: Seq[Index]): Unit = {
214213
val dataset = Utils.openDatasetBuilder(readOptions).build()
215214
try {
216-
val existingIndices = dataset.getIndexes.asScala
217-
.filter(_.name() == indexName)
218-
.toList
219-
220-
val committedSegments =
221-
dataset.commitExistingIndexSegments(
222-
indexName,
223-
column,
224-
segments.toList.asJava).asScala.toList
225-
226-
if (existingIndices.nonEmpty) {
227-
val committedUuids = committedSegments.map(_.uuid()).toSet
228-
val refreshedDataset = Utils.openDatasetBuilder(readOptions).build()
229-
try {
230-
val removedIndices = refreshedDataset.getIndexes.asScala
231-
.filter(index => index.name() == indexName && !committedUuids.contains(index.uuid()))
232-
.toList
233-
.asJava
234-
235-
if (!removedIndices.isEmpty) {
236-
val op = AddIndexOperation.builder()
237-
.withNewIndices(Collections.emptyList())
238-
.withRemovedIndices(removedIndices)
239-
.build()
240-
val txn = new Transaction.Builder()
241-
.readVersion(refreshedDataset.version())
242-
.operation(op)
243-
.build()
244-
try {
245-
val newDataset = new CommitBuilder(refreshedDataset)
246-
.writeParams(readOptions.getStorageOptions)
247-
.execute(txn)
248-
newDataset.close()
249-
} finally {
250-
txn.close()
251-
}
252-
}
253-
} finally {
254-
refreshedDataset.close()
255-
}
256-
}
215+
dataset.commitExistingIndexSegments(
216+
indexName, column, segments.toList.asJava)
257217
} finally {
258218
dataset.close()
259219
}

0 commit comments

Comments
 (0)