brucearctor opened a new pull request, #13891:
URL: https://github.com/apache/gravitino/pull/13891
### What changes were proposed in this pull request?
1. **Java GVFS (`FilesetMetadataCache` & `BaseGVFSOperations`)**:
- Widen `FilesetMetadataCache.catalogCache` from `Cache<NameIdentifier,
FilesetCatalog>` to `Cache<NameIdentifier, Catalog>`, add `public Catalog
getCatalog(NameIdentifier)`, and update `getFilesetCatalog`, `getFileset`, and
`getSchema` to reuse `getCatalog(catalogIdent)` inside their cache mapping
functions.
- Add `protected Catalog getCatalog(NameIdentifier)` to
`BaseGVFSOperations` and update `BaseGVFSOperations.getSchema(NameIdentifier)`
to lazily initialize `filesetMetadataCache` via `getFilesetMetadataCache()`
(fixing a `NullPointerException` when `enableFilesetMetadataCache=true`) and
route uncached catalog lookups through `getCatalog(catalogIdent)`.
- Route `BaseGVFSOperations.getAllProperties` through `getCatalog`,
`getSchema`, and `getFileset`, and pass the pre-loaded `Fileset` from
`getActualFileSystemByLocationName` to avoid reloading the fileset.
- Null-check `getCatalog(catalogIdent).properties()` in
`BaseGVFSOperations.createFilesetLocationIfNeed`.
2. **Python GVFS (`gvfs_base_operations.py`)**:
- Update `BaseGVFSOperations._merge_fileset_properties` to route catalog,
schema, and fileset lookups through `self._get_fileset_catalog`,
`self._get_fileset_schema`, and `self._get_fileset`, and accept an optional
pre-loaded `fileset` from `_get_actual_filesystem_by_location_name`.
3. **Documentation (`docs/how-to-use-gvfs.md`)**:
- Fix the configuration key typo on line 75
(`fs.gravitino.filesetMetadataCache.enable` ->
`fs.gravitino.filesetMetadataCache.cache.enable`) to match
`GravitinoVirtualFileSystemConfiguration.FS_GRAVITINO_FILESET_METADATA_CACHE_ENABLE`,
and align the description cells to include `fileset schema`.
### Why are the changes needed?
When `fs.gravitino.filesetMetadataCache.cache.enable=true` (Java) or
`enable_fileset_metadata_cache=True` (Python) is enabled, `getAllProperties` /
`_merge_fileset_properties` bypassed the metadata caches and issued uncached
`loadCatalog`, `loadSchema`, and `loadFileset` RPCs on every filesystem cache
miss. In addition, `BaseGVFSOperations.getSchema(NameIdentifier)` accessed
`this.filesetMetadataCache` directly without lazy initialization
(`getFilesetMetadataCache()`), throwing a `NullPointerException` when metadata
caching was enabled, and `FilesetMetadataCache.getSchema(NameIdentifier)`
bypassed `catalogCache`.
Fix: #13890
### Does this PR introduce _any_ user-facing change?
No. The documentation in `docs/how-to-use-gvfs.md` is updated to reflect the
actual configuration key (`fs.gravitino.filesetMetadataCache.cache.enable`).
### How was this patch tested?
- **Java unit tests**:
- `TestFilesetMetadataCache#testCacheReusesCatalogSchemaAndFileset`
-
`TestBaseGVFSOperationsSecrets#testGetAllPropertiesUsesFilesetMetadataCacheWhenEnabled`
-
`TestBaseGVFSOperationsSecrets#testGetAllPropertiesReusesPreloadedFileset`
- `TestBaseGVFSOperationsSecrets#testGetSchemaLazyInitializesMetadataCache`
- **Python unit & integration tests**:
- `test_gvfs_merge_secrets.py`:
`test_merge_properties_uses_fileset_metadata_cache_when_enabled` and
`test_merge_properties_reuses_preloaded_fileset`
- `test_gvfs_with_s3.py`: Clear `_schema_cache` alongside `_fileset_cache`
and `_catalog_cache` when resetting metadata cache in `test_mkdir`
--
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]
