Skip to content

Commit 41c6055

Browse files
committed
[fix](lance) Select FOR TIME AS OF by commit time and read with what the SDK opened
FOR TIME AS OF now compares commit times with one comparator throughout and no longer assumes they grow with version numbers. A version removed by cleanup takes its commit time with it, so, as Iceberg drops its snapshot log before a removed snapshot, only the versions newer than the newest removed one are candidates. For a managed table, every recorded version in that range that the storage listing lacks is checked out to read its commit time, which also finalizes a staged manifest. The selection lives in LanceSnapshotResolver and is checked against its rule on random histories. The Lance SDK describes a managed table again when it opens it and uses the location and options of that describe. The access handed to the BE is now rebuilt from what the SDK opened with, so both read the same location with the same configuration. If the SDK did not open with exactly the rebuilt access's options, as after an option's spelling changes or the table moves to another store, the dataset is opened once more with them. This replaces the relocation compare-and-retry.
1 parent 6341fcb commit 41c6055

7 files changed

Lines changed: 532 additions & 122 deletions

File tree

‎fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceCatalogClient.java‎

Lines changed: 46 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,10 @@
6161
import java.util.List;
6262
import java.util.Locale;
6363
import java.util.Map;
64-
import java.util.NavigableMap;
6564
import java.util.NavigableSet;
6665
import java.util.Objects;
6766
import java.util.Optional;
6867
import java.util.OptionalLong;
69-
import java.util.TreeMap;
7068
import java.util.TreeSet;
7169
import java.util.function.BiFunction;
7270
import java.util.regex.Pattern;
@@ -291,23 +289,6 @@ private LanceTableMetadata loadQueryMetadata(String dbName, String tableName,
291289
*/
292290
private <T> T readTableSnapshot(String dbName, String tableName, LanceRefSelector selector,
293291
SnapshotReader<T> reader) {
294-
try {
295-
return readTableSnapshotOnce(dbName, tableName, selector, reader);
296-
} catch (StaleTableAccessException e) {
297-
// The cached access predates a location change the SDK has already seen. Read once
298-
// more with a fresh access, so the FE plans and the BE reads the same location.
299-
namespaceClient.invalidateTableAccess(dbName, tableName);
300-
try {
301-
return readTableSnapshotOnce(dbName, tableName, selector, reader);
302-
} catch (StaleTableAccessException again) {
303-
throw new RuntimeException("Lance namespace reported a different location for " + dbName + "."
304-
+ tableName + " while it was being opened; retry the query");
305-
}
306-
}
307-
}
308-
309-
private <T> T readTableSnapshotOnce(String dbName, String tableName, LanceRefSelector selector,
310-
SnapshotReader<T> reader) {
311292
ReadState state = new ReadState(selector, dbName + "." + tableName);
312293
LanceMetadataMetrics metrics = LanceMetadataMetrics.startMetadataRead();
313294
try {
@@ -319,14 +300,14 @@ private <T> T readTableSnapshotOnce(String dbName, String tableName, LanceRefSel
319300
OptionalLong direct = directMainVersion(state, metrics);
320301
if (direct.isPresent() || isLatestMain(selector)) {
321302
state.version = direct;
322-
try (Dataset dataset = openDataset(allocator, state.access, direct, metrics)) {
303+
try (Dataset dataset = openDataset(allocator, state, direct, metrics)) {
323304
result = reader.read(dataset, state.access, metrics);
324305
}
325306
} else {
326307
OptionalLong mainVersion = state.access.isManagedVersioning()
327308
? OptionalLong.of(recordedLatestVersion(state, Optional.empty(), metrics))
328309
: OptionalLong.empty();
329-
try (Dataset main = openDataset(allocator, state.access, mainVersion, metrics)) {
310+
try (Dataset main = openDataset(allocator, state, mainVersion, metrics)) {
330311
result = readFromLatest(main, state, reader, metrics);
331312
}
332313
}
@@ -335,8 +316,6 @@ private <T> T readTableSnapshotOnce(String dbName, String tableName, LanceRefSel
335316
return result;
336317
} catch (LanceUserFacingException e) {
337318
throw new RuntimeException(e.getMessage(), e);
338-
} catch (StaleTableAccessException e) {
339-
throw e;
340319
} catch (Exception e) {
341320
LanceTableAccess access = state.access;
342321
String uri = access == null ? null : access.getDatasetUri();
@@ -508,13 +487,6 @@ private LanceUserFacingException(String message) {
508487
}
509488
}
510489

511-
/** The SDK opened a managed table at another location than the resolved access names. */
512-
private static final class StaleTableAccessException extends RuntimeException {
513-
private StaleTableAccessException() {
514-
super("Lance table location changed since its access was resolved");
515-
}
516-
}
517-
518490
/**
519491
* The newest version the namespace records for a managed chain. Doris asks for it itself:
520492
* opening "latest" through the SDK falls back to the newest manifest in storage when the
@@ -636,14 +608,10 @@ private List<TableVersion> namespaceVersions(ReadState state, LanceTableAccess a
636608

637609
/**
638610
* Resolves {@code FOR TIME AS OF} to a version on the chain {@code latest} is checked out on,
639-
* from the commit times the manifests record, as Lance resolves {@code asof}. A managed table
640-
* only selects among the versions its namespace records.
641-
*
642-
* <p>The version next to the selection must still exist: if the version after it, or with
643-
* nothing listed at or before the time the one before the oldest listed, was removed by
644-
* cleanup, the table's state at that time is unknown and the query fails instead of reading
645-
* an older snapshot. A recorded version missing from the storage listing because its manifest
646-
* is still staged is checked out there, which finalizes it and yields its commit time.
611+
* from the commit times the manifests record, over the history {@link LanceSnapshotResolver}
612+
* describes. A managed table only selects among the versions its namespace records; one
613+
* missing from the storage listing because its manifest is still staged is checked out, which
614+
* finalizes it and yields its commit time.
647615
*/
648616
private long resolveVersionAtOrBefore(Dataset latest, LanceTableAccess access, long timestamp,
649617
String requestedText, ReadState state, LanceMetadataMetrics metrics) {
@@ -652,64 +620,18 @@ private long resolveVersionAtOrBefore(Dataset latest, LanceTableAccess access, l
652620
.filter(Objects::nonNull).collect(Collectors.toCollection(TreeSet::new))
653621
: null;
654622
long version = metrics.measure(Stage.VERSION_RESOLVE, () -> {
655-
NavigableMap<Long, Version> listed = new TreeMap<>();
656-
for (Version candidate : latest.listVersions()) {
657-
if (recorded == null || recorded.contains(candidate.getId())) {
658-
listed.put(candidate.getId(), candidate);
659-
}
660-
}
661-
long selected;
662623
try {
663-
selected = LanceSnapshotResolver.versionAtOrBefore(listed.values(), timestamp, requestedText);
664-
} catch (LanceSnapshotResolver.NoVersionAtOrBeforeException e) {
665-
if (recorded == null) {
666-
throw e;
667-
}
668-
// Recorded versions older than the oldest listed one may still be staged. Walking
669-
// back, every version passed was committed after the time, so the first one at or
670-
// before it is the answer.
671-
Long older = listed.isEmpty() ? recorded.last() : recorded.lower(listed.firstKey());
672-
while (older != null) {
673-
Version olderVersion = recordedVersion(latest, access, older);
674-
if (olderVersion == null) {
675-
throw historyRemoved(older, requestedText, state);
676-
}
677-
if (LanceSnapshotResolver.commitMillis(olderVersion) <= timestamp) {
678-
return older;
679-
}
680-
older = recorded.lower(older);
681-
}
682-
throw e;
683-
}
684-
// Lance numbers a chain's commits consecutively, so a storage chain is every number
685-
// between the oldest and newest listed manifest; a managed chain is what is recorded.
686-
Long next = nextVersion(selected, recorded, listed);
687-
while (next != null) {
688-
Version nextVersion = listed.containsKey(next) ? listed.get(next)
689-
: recorded == null ? null : recordedVersion(latest, access, next);
690-
if (nextVersion == null) {
691-
throw historyRemoved(next, requestedText, state);
692-
}
693-
if (LanceSnapshotResolver.commitMillis(nextVersion) > timestamp) {
694-
break;
695-
}
696-
selected = next;
697-
next = nextVersion(next, recorded, listed);
624+
return LanceSnapshotResolver.versionAtOrBefore(latest.listVersions(), recorded,
625+
id -> recordedVersion(latest, access, id), timestamp, requestedText);
626+
} catch (LanceSnapshotResolver.HistoryRemovedException e) {
627+
throw historyRemoved(e.getVersion(), requestedText, state);
698628
}
699-
return selected;
700629
});
701630
LOG.debug("Resolved Lance FOR TIME AS OF '{}' to version {} from manifest commit times", requestedText,
702631
version);
703632
return version;
704633
}
705634

706-
private static Long nextVersion(long version, NavigableSet<Long> recorded, NavigableMap<Long, Version> listed) {
707-
if (recorded != null) {
708-
return recorded.higher(version);
709-
}
710-
return version < listed.lastKey() ? version + 1 : null;
711-
}
712-
713635
/**
714636
* A namespace-recorded version checked out through the namespace, or null if it is gone. A
715637
* still-staged manifest is finalized by the checkout.
@@ -769,21 +691,46 @@ private static boolean isVersionNotFound(Throwable throwable) {
769691
|| (lower.contains("not found") && lower.contains("_versions/"));
770692
}
771693

772-
private Dataset openDataset(BufferAllocator allocator, LanceTableAccess access, OptionalLong version,
694+
/**
695+
* Opens the main chain of the table {@code state} resolved. For a managed table the SDK
696+
* describes the table again and opens the location and storage options that describe returns,
697+
* so {@code state.access} is replaced by the access for what it opened: the BE reads with the
698+
* access this read ends up with, and must open what the FE planned. If the SDK did not open
699+
* with exactly that access's options, the dataset is opened once more with them. A namespace
700+
* that returns a relative location cannot be read in this mode.
701+
*/
702+
private Dataset openDataset(BufferAllocator allocator, ReadState state, OptionalLong version,
773703
LanceMetadataMetrics metrics) {
774-
ReadOptions readOptions = LanceReadOptions.forSharedSession(access.getSdkStorageOptions(), version, session);
775-
if (access.isManagedVersioning()) {
776-
// The SDK re-describes the table and opens the location the namespace returns, so a
777-
// namespace that returns a relative location cannot be read in this mode. The BE
778-
// opens the access's location, so both must still agree after that second describe.
779-
Dataset dataset = metrics.measure(Stage.DATASET_OPEN,
780-
() -> namespaceClient.openManagedDataset(allocator, access, readOptions, session));
781-
if (!StringUtils.removeEnd(dataset.uri(), "/").equals(StringUtils.removeEnd(access.getDatasetUri(), "/"))) {
704+
if (state.access.isManagedVersioning()) {
705+
LanceTableAccess access = state.access;
706+
for (int attempt = 0; ; attempt++) {
707+
ReadOptions readOptions = LanceReadOptions.forSharedSession(access.getSdkStorageOptions(), version,
708+
session);
709+
LanceTableAccess requested = access;
710+
Dataset dataset = metrics.measure(Stage.DATASET_OPEN,
711+
() -> namespaceClient.openManagedDataset(allocator, requested, readOptions, session));
712+
boolean opensAsBuilt;
713+
try {
714+
Map<String, String> openedOptions = dataset.getInitialStorageOptions();
715+
access = namespaceClient.accessOpenedBySdk(requested, dataset.uri(), openedOptions);
716+
opensAsBuilt = LanceNamespaceClient.opensAs(access, dataset.uri(), openedOptions);
717+
} catch (RuntimeException e) {
718+
dataset.close();
719+
throw e;
720+
}
721+
if (opensAsBuilt) {
722+
state.access = access;
723+
return dataset;
724+
}
782725
dataset.close();
783-
throw new StaleTableAccessException();
726+
if (attempt > 0) {
727+
throw new LanceUserFacingException("Lance namespace changed the location or storage options of "
728+
+ state.tableName + " while it was being opened; retry the query");
729+
}
784730
}
785-
return dataset;
786731
}
732+
LanceTableAccess access = state.access;
733+
ReadOptions readOptions = LanceReadOptions.forSharedSession(access.getSdkStorageOptions(), version, session);
787734
return metrics.measure(Stage.DATASET_OPEN, () -> Dataset.open().allocator(allocator).uri(access.getDatasetUri())
788735
.readOptions(readOptions).build());
789736
}

‎fe/fe-core/src/main/java/org/apache/doris/datasource/lance/LanceNamespaceClient.java‎

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import java.util.ArrayDeque;
5252
import java.util.ArrayList;
5353
import java.util.Collections;
54+
import java.util.HashMap;
5455
import java.util.HashSet;
5556
import java.util.LinkedHashSet;
5657
import java.util.List;
@@ -221,11 +222,6 @@ LanceTableAccess resolveTableAccessUncached(String dbName, String tableName) {
221222
return loadTableAccess(tableAccessKey(dbName, tableName)).access;
222223
}
223224

224-
/** Drops one table's cached access, so its next read describes the table again. */
225-
void invalidateTableAccess(String dbName, String tableName) {
226-
tableAccessCache.invalidate(tableAccessKey(dbName, tableName));
227-
}
228-
229225
void invalidateTableAccessCache() {
230226
// Swap generations: a describe already in flight may finish for its caller, but must
231227
// never repopulate the cache used by reads admitted after an explicit refresh.
@@ -250,11 +246,6 @@ private CachedTableAccess loadTableAccess(List<String> tableId) {
250246
throw new RuntimeException("Lance namespace returned no table URI for " + tableId);
251247
}
252248

253-
// One option map serves both readers: the FE opens the dataset through the Lance Java SDK
254-
// and the BE through lance-c, so neither can end up with credentials the other lacks. The
255-
// dataset URL picks the option vocabulary, the same way Lance picks a provider from it.
256-
Map<String, String> storageOptions = LanceStorageOptions.fromDorisAndVendedStorageOptions(datasetUri,
257-
storageProperties, table.getStorageOptions());
258249
LanceTableAccess access;
259250
if (Boolean.TRUE.equals(table.getManagedVersioning())) {
260251
// The namespace, not the dataset directory, records which manifest each version has.
@@ -270,15 +261,70 @@ private CachedTableAccess loadTableAccess(List<String> tableId) {
270261
throw new RuntimeException("Lance namespace returned a table_uri that differs from location for "
271262
+ "managed table " + tableId);
272263
}
273-
access = LanceTableAccess.managedByNamespace(datasetUri, storageOptions,
274-
LanceStorageOptions.forManagedSdkOpen(datasetUri, storageOptions, table.getStorageOptions()),
275-
tableId);
264+
access = managedAccess(datasetUri, table.getStorageOptions(), tableId);
276265
} else {
277-
access = new LanceTableAccess(datasetUri, storageOptions);
266+
access = new LanceTableAccess(datasetUri, storageOptions(datasetUri, table.getStorageOptions()));
278267
}
279268
return new CachedTableAccess(access, tableAccessTtlNanos(datasetUri, table.getStorageOptions()));
280269
}
281270

271+
/**
272+
* One option map serves both readers: the FE opens the dataset through the Lance Java SDK and
273+
* the BE through lance-c, so neither can end up with credentials the other lacks. The dataset
274+
* URL picks the option vocabulary, the same way Lance picks a provider from it.
275+
*/
276+
private Map<String, String> storageOptions(String datasetUri, Map<String, String> vendedOptions) {
277+
return LanceStorageOptions.fromDorisAndVendedStorageOptions(datasetUri, storageProperties, vendedOptions);
278+
}
279+
280+
LanceTableAccess managedAccess(String datasetUri, Map<String, String> vendedOptions,
281+
List<String> tableId) {
282+
Map<String, String> vended = vendedOptions == null ? Collections.emptyMap() : vendedOptions;
283+
Map<String, String> storageOptions = storageOptions(datasetUri, vended);
284+
return LanceTableAccess.managedByNamespace(datasetUri, storageOptions,
285+
LanceStorageOptions.forManagedSdkOpen(datasetUri, storageOptions, vended), vended, tableId);
286+
}
287+
288+
/**
289+
* The access that reads what the SDK opened for a managed table. The SDK describes the table
290+
* again and opens the location and vended options of that describe, so if the namespace
291+
* changed either since {@code access} was resolved, the BE must be handed the new ones: the
292+
* FE plans the dataset the SDK opened, and the BE has to open the same one. A cached access
293+
* is left as it is and expires with its TTL; until then every read derives the new one again.
294+
*
295+
* @param openedOptions the options the SDK opened with, as {@code Dataset.getInitialStorageOptions()}
296+
* reports them: the options it was handed with its describe's vended options put on top
297+
*/
298+
LanceTableAccess accessOpenedBySdk(LanceTableAccess access, String openedUri, Map<String, String> openedOptions) {
299+
Map<String, String> opened = openedOptions == null ? Collections.emptyMap() : openedOptions;
300+
boolean sameUri = StringUtils.removeEnd(openedUri, "/")
301+
.equals(StringUtils.removeEnd(access.getDatasetUri(), "/"));
302+
if (sameUri && opened.equals(access.getSdkStorageOptions())) {
303+
return access;
304+
}
305+
// Every entry that differs from what the SDK was handed came from its describe. What the
306+
// namespace vended the first time and not again stays, as it does for the SDK.
307+
Map<String, String> vended = new HashMap<>(access.getVendedStorageOptions());
308+
opened.forEach((key, value) -> {
309+
if (!value.equals(access.getSdkStorageOptions().get(key))) {
310+
vended.put(key, value);
311+
}
312+
});
313+
return managedAccess(sameUri ? access.getDatasetUri() : openedUri, vended, access.getNamespaceTableId());
314+
}
315+
316+
/**
317+
* Whether the SDK opened {@code openedUri} with exactly the options {@code access} hands it, so
318+
* the FE read what the BE will. It did not when the namespace changed an option's spelling or
319+
* a value an option is inferred from, or moved the table to another store: the SDK then still
320+
* holds the old spelling or vocabulary next to the new one.
321+
*/
322+
static boolean opensAs(LanceTableAccess access, String openedUri, Map<String, String> openedOptions) {
323+
Map<String, String> opened = openedOptions == null ? Collections.emptyMap() : openedOptions;
324+
return StringUtils.removeEnd(openedUri, "/").equals(StringUtils.removeEnd(access.getDatasetUri(), "/"))
325+
&& opened.equals(access.getSdkStorageOptions());
326+
}
327+
282328
/**
283329
* Opens a namespace-managed dataset through the namespace client. The SDK describes the table
284330
* through this Java client, outside {@code namespaceLock}, and then resolves versions with its

0 commit comments

Comments
 (0)