Uniqueness violations during check version appear to be silently ignored

[alecgrieser]

If a new unique index is added to a store and that index can be built in-line during checkVersion, it appears that this can result in the index being built and marked as readable even if there are uniqueness violations. I believe this to be caused by the way that index states are handled, as the new indexes are not marked as write-only before building them, but then there's an optimization in markIndexReadable that is intended to allow us to skip some work if someone marks an already readable index as readable:

fdb-record-layer/fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBRecordStore.java

Lines 2733 to 2764 in 2cd915c

	byte[] indexKey = indexStateSubspace().pack(index.getName());
	CompletableFuture<Boolean> future = tr.get(indexKey).thenCompose(previous -> {
	if (previous != null) {
	CompletableFuture<Optional<Range>> builtFuture = firstUnbuiltRange(index);
	CompletableFuture<Optional<RecordIndexUniquenessViolation>> uniquenessFuture = scanUniquenessViolations(index, 1).first();
	return CompletableFuture.allOf(builtFuture, uniquenessFuture).thenApply(vignore -> {
	Optional<Range> firstUnbuilt = context.join(builtFuture);
	Optional<RecordIndexUniquenessViolation> uniquenessViolation = context.join(uniquenessFuture);
	if (firstUnbuilt.isPresent()) {
	throw new IndexNotBuiltException("Attempted to make unbuilt index readable" , firstUnbuilt.get(),
	LogMessageKeys.INDEX_NAME, index.getName(),
	"unbuiltRangeBegin", ByteArrayUtil2.loggable(firstUnbuilt.get().begin),
	"unbuiltRangeEnd", ByteArrayUtil2.loggable(firstUnbuilt.get().end),
	subspaceProvider.logKey(), subspaceProvider.toString(context),
	LogMessageKeys.SUBSPACE_KEY, index.getSubspaceKey());
	} else if (uniquenessViolation.isPresent()) {
	RecordIndexUniquenessViolation wrapped = new RecordIndexUniquenessViolation("Uniqueness violation when making index readable",
	uniquenessViolation.get());
	wrapped.addLogInfo(
	LogMessageKeys.INDEX_NAME, index.getName(),
	subspaceProvider.logKey(), subspaceProvider.toString(context));
	throw wrapped;
	} else {
	updateIndexState(index.getName(), indexKey, IndexState.READABLE);
	clearReadableIndexBuildData(tr, index);
	return true;
	}
	});
	} else {
	return AsyncUtil.READY_FALSE;
	}
	}).whenComplete((b, t) -> endRecordStoreStateWrite()).thenApply(this::addRemoveReplacedIndexesCommitCheckIfChanged);

In the case of an index during checkVersion, the index won't have a previous state, so the logic to check if there are any uniqueness violations will be skipped, which means the index will be marked as readable even though it has uniqueness violations.