JingsongLi commented on code in PR #8356:
URL: https://github.com/apache/paimon/pull/8356#discussion_r3475107897
##########
paimon-core/src/main/java/org/apache/paimon/table/query/LocalTableQuery.java:
##########
@@ -192,28 +198,53 @@ private void newLookupLevels(BinaryRow partition, int
bucket, List<DataFileMeta>
this.options.lookupRemoteLevelThreshold());
}
- tableView.computeIfAbsent(partition, k -> new HashMap<>()).put(bucket,
lookupLevels);
+ return lookupLevels;
+ }
+
+ private Cache<String, LookupFile> lookupFileCache(Options options) {
+ Cache<String, LookupFile> cache = lookupFileCache;
+ if (cache == null) {
+ synchronized (this) {
+ cache = lookupFileCache;
+ if (cache == null) {
+ cache =
+ LookupFile.createCache(
+
options.get(CoreOptions.LOOKUP_CACHE_FILE_RETENTION),
+
options.get(CoreOptions.LOOKUP_CACHE_MAX_DISK_SIZE));
+ lookupFileCache = cache;
+ }
+ }
+ }
+ return cache;
}
- /** TODO remove synchronized and supports multiple thread to lookup. */
@Nullable
@Override
- public synchronized InternalRow lookup(BinaryRow partition, int bucket,
InternalRow key)
- throws IOException {
- Map<Integer, LookupLevels<KeyValue>> buckets =
tableView.get(partition);
+ public InternalRow lookup(BinaryRow partition, int bucket, InternalRow
key) throws IOException {
Review Comment:
By removing the method-level synchronization here, cache misses for
different files in the same bucket can now enter `LookupLevels` concurrently.
Those paths share the same `KeyValueFileReaderFactory`, whose
`formatReaderMappings` cache is a plain `HashMap` mutated via `computeIfAbsent`
during `createRecordReader`. The old synchronized `lookup` serialized that
path; this change exposes the shared factory to concurrent mutation. Please
make that cache thread-safe or serialize reader creation for the shared factory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
