[common] Version the memory-mode cache key in CachingFileIO - #9642
Open
LuciferYang wants to merge 1 commit into
Open
[common] Version the memory-mode cache key in CachingFileIO#9642LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
The memory branch of newInputStream keyed the cache by path alone while the disk branch keys by path, length and modification time. consumer-* and service-* classify as META, which the default whitelist caches, and both are written in place by overwriteFileUtf8, so a consumer reset kept serving the pre-reset content. Build the same versioned key for the memory branch, at the cost of one getFileStatus per open there.
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
1 participant
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
close #9641
CachingFileIO.newInputStreamkeyed the memory cache by path alone, while the disk branch keys by path, length and modification time. The whitelist that decides what gets cached containsMETA, andconsumer-*andservice-*classify asMETAwhile being exactly the files written in place byoverwriteFileUtf8.FileType.isMutableonly excludesEARLIESTandLATEST, so after a consumer reset the cached blocks keep serving the pre-reset content, with the cached file size pinned per path too.The memory branch now builds the same versioned key the disk branch uses, keeping the namespace prefix that
SharedCacheManagerinvalidation matches on, and passes the known length instead of-1.Blacklisting the two prefixes in
isMutablewould have been cheaper, and I went the other way deliberately:TagManager.createOrReplaceTagoverwritestag-*, the Iceberg metadata writes go throughoverwriteFileUtf8, and so does_SUCCESS, so a prefix list is something to keep maintaining while a version in the key is not.The cost is one
delegate.getFileStatusper open in memory mode, a HEAD on an object store. Before this, the size was resolved lazily on first read and then cached per path, so the change is from once per path to once per open; cached blocks are unaffected either way. One limit remains, shared with the disk mode: a delegate with second-granularity modification times can still collide if a rewrite lands in the same second at the same length.Tests
CachingFileIOTest.testMemoryModeServesFreshContentAfterInPlaceOverwritereadsconsumer-1through a memory-modeCachingFileIO, replaces the file in place with different content and a later modification time, and reads again. It asserts the content of both reads and the number of times the delegate was opened, so it pins both freshness and that the first version really was served from cache.Against the unfixed code the second read returns the first version's bytes.
mvn -pl paimon-common -Dtest=CachingFileIOTest teston JDK 8: 29 tests, 0 failures.spotless:checkandcheckstyle:checkon paimon-common are clean.