[Core]add table schema cache for SchemaManager#2939
Closed
wg1026688210 wants to merge 9 commits into
Closed
Conversation
JingsongLi
reviewed
Mar 3, 2024
| * The class is responsible for providing a schemaManager with a concurrent and serializable schema | ||
| * cache. | ||
| */ | ||
| public class SchemaCache implements Serializable { |
Contributor
There was a problem hiding this comment.
You don't need to introduce this class, just merge this into SchemaManager.
JingsongLi
reviewed
Mar 3, 2024
| } | ||
|
|
||
| private void writeObject(ObjectOutputStream out) throws IOException { | ||
| Map<Long, TableSchema> map = new HashMap<>(cache.asMap()); |
Contributor
There was a problem hiding this comment.
I think we don't need serialize this cache.
wg1026688210
force-pushed
the
core/schema_cache
branch
4 times, most recently
from
0aa4f0b to
6c6e483
Compare
wg1026688210
force-pushed
the
core/schema_cache
branch
from
6c6e483 to
7b5b862
Compare
wg1026688210
force-pushed
the
core/schema_cache
branch
from
7b5b862 to
a013cad
Compare
JingsongLi
reviewed
Mar 6, 2024
| private Map<Long, TableSchema> loadSchemaCache(FileIO fileIO, Path path) { | ||
| Map<Long, TableSchema> schemaCache = new ConcurrentHashMap<>(); | ||
| SchemaManager schemaManager = new SchemaManager(fileIO, path); | ||
| for (TableSchema schema : schemaManager.listAll()) { |
Contributor
There was a problem hiding this comment.
Why we need to list all at first?
JingsongLi
reviewed
Mar 6, 2024
| protected final TableSchema tableSchema; | ||
| protected final CatalogEnvironment catalogEnvironment; | ||
|
|
||
| protected final Map<Long, TableSchema> schemaCache; |
Contributor
There was a problem hiding this comment.
or just store SchemaManager here?
2 tasks
wg1026688210
force-pushed
the
core/schema_cache
branch
from
7df4bf1 to
9608263
Compare
wg1026688210
changed the title
JingsongLi
reviewed
Mar 11, 2024
| private final Map<Long, TableSchema> cache; | ||
|
|
||
| public SchemaManager(FileIO fileIO, Path tableRoot) { | ||
| this(fileIO, tableRoot, null); |
Contributor
There was a problem hiding this comment.
new hash map? just keep cache not null?
JingsongLi
reviewed
Mar 11, 2024
| } | ||
| this.tableSchema = tableSchema; | ||
| this.catalogEnvironment = catalogEnvironment; | ||
| tableSchemaManager = new SchemaManager(fileIO, path, new ConcurrentHashMap<>()); |
Contributor
There was a problem hiding this comment.
Just use public SchemaManager(FileIO fileIO, Path tableRoot)
JingsongLi
reviewed
Mar 11, 2024
| manager.commitChanges(SchemaChange.setOption("ccc", "ddd")); | ||
|
|
||
| Map<Long, TableSchema> cachedSchema = manager.getCachedSchema(); | ||
| assertThat(cachedSchema).hasSize(3); |
Contributor
There was a problem hiding this comment.
You don't need to getCachedSchema.
You can just verify same instances.
wg1026688210
force-pushed
the
core/schema_cache
branch
from
428f5bb to
3ae9815
Compare
Contributor
|
See #3021 |
Contributor
|
At present, this solution is relatively obscure. Generally speaking, it is difficult to accept a solution where the cache is serialized and reused by distributed tasks. Considering that #3021 has already been merged and can solve most problems in most cases (without schema changes), I am considering closing this PR. You can reopen this PR at any time if you have any further needs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
When reading a split ,recordReader will loads the schema of the split from the FileSystem.
The pr is for adding the cache of TableSchema for SchemaManager to reduce the access of FileSystem.
Tests
SchemaManagerTest#testCache
API and Format
Documentation