Remove CallContext #2556
Draft
Remove CallContext #2556
Conversation
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
XN137
force-pushed
the
remove-CallContext
branch
3 times, most recently
from
453e3a3 to
9db523a
Compare
XN137
force-pushed
the
remove-CallContext
branch
8 times, most recently
from
a4ea060 to
d1f6dff
Compare
XN137
force-pushed
the
remove-CallContext
branch
4 times, most recently
from
8b17318 to
5c80acb
Compare
XN137
force-pushed
the
remove-CallContext
branch
4 times, most recently
from
15ea1f1 to
cf25520
Compare
XN137
force-pushed
the
remove-CallContext
branch
3 times, most recently
from
114cc94 to
3601564
Compare
the `PolarisMetaStoreManager` interface methods all take a `PolarisCallContext` parameter because they need access to `PolarisCallContext.getMetaStore` aka the persistence session. also occasionally the `RealmContext` and `RealmConfig` is needed. this means `PolarisMetaStoreManager` is only usable within a request-scope. `MetaStoreManagerFactory.getOrCreateMetaStoreManager` impls suggests that there needs to be one shared `PolarisMetaStoreManager` instance per realm. however if we look at the `PolarisMetaStoreManager` impls (`AtomicOperationMetaStoreManager` and `TransactionalMetaStoreManagerImpl`) we can see that they dont have any state themselves, the fields only reference application-scoped collaborators. those 2 observations combined mean that in order to reduce the dependency on `PolarisCallContext` the `PolarisMetaStoreManager` has to become request-scoped. note that if custom implementations would need any state sharing for a realm, they are still free to do so in their custom `MetaStoreManagerFactory` implementations, as they can initialize the request-scoped `PolarisMetaStoreManager` with whatever data they need. when `PolarisMetaStoreManager` is request-scoped the `getOrCreateSession` method also becomes a private implementation detail of the `MetaStoreManagerFactory`. the `EntityCache` / `InMemoryEntityCache` however can no longer operate on a single `PolarisMetaStoreManager` and the one from the request needs to be passed as a parameter in all the methods now.
this avoids `TokenBrokerFactory` impls having to call `MetaStoreManagerFactory.createMetaStoreManager`
since `PolarisCredentialVendor` (a sub-interface of `PolarisMetaStoreManager`) no longer has a `CallContext` parameter we can see a domino effect: - `StorageCredentialCache` no longer needs `CallContext` - `TokenBroker` no longer needs `CallContext` - `FileIOFactory` no longer needs `CallContext` - `TaskFileIOSupplier` no longer needs `CallContext` - `TaskHandler` and `TaskExecutor` no longer need `CallContext` - `ResolverFactory` and `ResolutionManifest` no longer need `CallContext` and eventually `PolarisCallContext` and `CallContext` are completely unused and can be removed or replaced by passing `RealmContext` and `RealmConfig` individually.
XN137
force-pushed
the
remove-CallContext
branch
from
3601564 to
1dd1f94
Compare
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.
WIP as currently based on https://github.com/apache/polaris/pull/2555since
PolarisCredentialVendor(a sub-interface ofPolarisMetaStoreManager) no longer hasa
CallContextparameter we can see a domino effect:StorageCredentialCacheno longer needsCallContextTokenBrokerno longer needsCallContextFileIOFactoryno longer needsCallContextTaskFileIOSupplierno longer needsCallContextTaskHandlerandTaskExecutorno longer needCallContextResolverFactoryandResolutionManifestno longer needCallContextand eventually
PolarisCallContextandCallContextare completely unused and can beremoved or replaced by passing
RealmContextandRealmConfigindividually.