pingtimeout commented on issue #761: URL: https://github.com/apache/polaris/issues/761#issuecomment-2765415736
> you could try using the IndexedCache example to see if it would pass your test AFAICT, it would make things better. But probably not solve all issues (see below). > the EntityCache here is actually not expected to be the component responsible for strict consistency I would argue that it should at least be consistent with itself. But the way it is currently written makes it not. In its current design, `EntityCache` sometimes is used with a read-through strategy. In the `getOrLoad...` methods, if an entity is null, the cache reads from the database and populates itself before returning the entity. And it is also _sometimes_ used with a hybrid cache-aside-read-through strategy (in the `bulkValidate` method). The `Resolver` class may reject the result, fetch data from the database, optionally invalidate the cache (which is cache aside) and go back to a read-through strategy by calling `EntityCache::getAndRefreshIfNeeded`. The combination of both strategy makes things quite tricky to reason about, let alone to fully check for correctness. > the out-of-order or stale cache entries here shouldn't be a problem 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. It should also be possible for a race condition to result in the EntityCache returning stale data with `isCacheHit == false`. In that case, the Resolver assumes that the entity is fresh and stale data is returned. > the original assumptions filed in this issue don't quite match what the EntityCache is doing. I wonder whether the methods `getEntityById` and `getEntityByName` should be private. They are *actual* cache-aside implementation methods, which is not what is used in the code. And they are only used by `EntityCache` itself, in a read-through fashion. On a more general note, I think the design of EntityCache+Resolver should be clearly stated. Example of questions to guide the design description: * Which caching strategy is intended? * What are the consistency expectations? * What are the eviction criteria? * ... From there, a thorough code review could surface many issues. -- 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]
