[core] Add cache for snapshots in CachingCatalog #4565
Conversation
JingsongLi
changed the title
JingsongLi
changed the title
| SNAPSHOT_CACHE.put(path, snapshot); | ||
| } | ||
| return snapshot; | ||
| } catch (FileNotFoundException e) { |
There was a problem hiding this comment.
May be we can only catch IOException? There's no need to catch FileNotFoundException and throw it direct?
There was a problem hiding this comment.
tryFromPath is just for FileNotFoundException, invoker can know the file does not exists.
| invalidateMetaCacheForPrefix(SCHEMA_CACHE, path); | ||
| } | ||
|
|
||
| private static void invalidateMetaCacheForPrefix(Cache<Path, ?> cache, String tablePath) { |
There was a problem hiding this comment.
May be invalidateMetaCacheByPrefix?
| import org.apache.paimon.schema.TableSchema; | ||
| import org.apache.paimon.tag.Tag; | ||
|
|
||
| /** Cache for {@link Snapshot} and {@link Tag} and {@link TableSchema}. */ |
There was a problem hiding this comment.
Is this class reserved for future use?
| @@ -680,6 +681,9 @@ protected void dropTableImpl(Identifier identifier) { | |||
| false, | |||
| true)); | |||
|
|
|||
| Path path = getTableLocation(identifier); | |||
| invalidateMetaCacheForPrefix(path); | |||
There was a problem hiding this comment.
May invalidateMetaCacheForPrefix(path) can execute in AbstractCatalog#dropTable after dropTableImpl(identifier)?
Because If a new Catalog @OverRide AbstractCatalog#dropTableImpl, it may forget to call invalidateMetaCacheForPrefix(path).
| } | ||
| } | ||
|
|
||
| public static Tag tryFromPath(FileIO fileIO, Path path) throws FileNotFoundException { |
There was a problem hiding this comment.
Why not cache Tag and Changelog like Snapshot？
There was a problem hiding this comment.
Tag is mutable, it is danger.
JingsongLi
changed the title
|
mark! |
Purpose
Considering that all snapshot files are immutable, we are considering adding cache to them. It is safe to cache them in Table Object.
Tests
API and Format
Documentation