dennishuo commented on issue #761:
URL: https://github.com/apache/polaris/issues/761#issuecomment-2790481548
Ah sorry @pingtimeout I didn't see your comments last week, there are some
points I can help clarify:
> Because of (2), any stress test that concurrently call different methods
of Resolver would be irrelevant. The code does not allow that pattern to
happen. In a way, it is a good thing because of the combination of (1) and (2).
I meant to indicate we need to stress test the architected control-flow
pattern. To clarify, when I said "run stress tests against the Resolver layer"
I mean that "layer" in the architectural stack, *not* reuse a single Resolver
instance.
Indeed, Resolver instances are single-use. The control-flow pattern is that
every concurrent request instantiates its own Resolver instance that uses the
shared cache.
> Well... Until you notice that bulkValidate assumes that an entity must be
fresh after EntityCache::getAndRefreshIfNeeded has been invoked. And the
jcstress tests show that getAndRefreshIfNeeded can go back in time.
This is actually two separate types of conditions. One is the definition of
"fresh" -- in `Resolver`, fresh means for a given Resolver *instance*, versions
must not go backwards in time.
This is distinct from whether or not *multiple* independent Resolver
instances might disagree between each other about whether something went
backwards in time.
The important thing is that these things come from the read-through
"changeTrackingVersions" of the underlying metastore:
if (versions == null
|| entity.getEntityVersion() != versions.getEntityVersion()
|| entity.getGrantRecordsVersion() !=
versions.getGrantRecordsVersion()) {
Thus, whenever the Resolver detects staleness in `bulkValidate`, the new
"target version" *must* be forwards in time (incidentally, perhaps an unstated
constraint is that entityVersions and grantRecordsVersions themselves must
monotonically increase).
Then, as long as the constraint for "minimumVersion" in
refreshedResolvedEntity =
this.cache.getAndRefreshIfNeeded(
this.polarisCallContext,
entity,
versions.getEntityVersion(),
versions.getGrantRecordsVersion());
holds true, it doesn't matter if other resolver instances saw "even newer"
versions than whatever is returned as `refreshedResolvedEntity`, as long as
`refreshedResolvedEntity` has version >= `versions.getEntityVersion()` and we
already know `versions.getEntityVersion() > entity.getEntityVersion()` by
virtue of the read-through change-tracking.
That said, it probably doesn't hurt to make the cache also make
`getAndRefreshIfNeeded` ensure *globally* monotonically increasing entity
versions *if* it's not too complex to guarantee.
--
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]
