dennishuo opened a new pull request, #1134:
URL: https://github.com/apache/polaris/pull/1134
Fix a bug in loadTasks which wasn't abiding by compare-and-swap semantics
when updating TaskEntities
Additionally, fix the PolarisTreeMapMetaStoreSessionImpl to avoid "cheating"
by using its entire PolarisBaseEntit it happens to store in entitiesActive when
performing a listing; in general, entitiesActive is assumed to *only* contain
the name lookup records, so it is not explicitly updated unless a parent or
name changes.
The existing tests happened to pass because of a coincidence of several bugs:
1. Since the entitiesActive and entities happens to share the same
PolarisBaseEntity instance by-reference, any in-place edits to "entities"
*happens* to reflect in entitiesActive, but in general entities *should not be
mutable* and usually a copy of an entity is mutated instead of mutating in
place anyways
2. Because of the blind-writes in loadTasks, the "prepare entity" step where
an instance is updated with a new entityVersion coincidentally immediately
causes the main entities *and* entitiesAcitve slices to have the updated entity
even before the call to writeEntity happens
3. Since TreeMap uses "synchronized" for runInTransaction, the
loadTasksInParallel doesn't detect interleaving of threads
So without the fix in the treemap store, simply fixing
PolarisMetaStoreManagerImpl causes the test to run forever because tasks never
get updated in entitiesActive so the same initial 5 tasks keep getting listed
forever.
Also fix the test to actually timeout eventually; the Future::get calls were
willing to block forever before getting to the await. Reduce the wait time from
10 minutes to something more reasonable.
<!--
Possible security vulnerabilities: STOP here and contact
[email protected] instead!
Please update the title of the PR with a meaningful message - do not
leave it "empty" or "generated"
Please update this summary field:
The summary should cover these topics, if applicable:
* the motivation for the change
* a description of the status quo, for example the current behavior
* the desired behavior
* etc
PR checklist:
- Do a self-review of your code before opening a pull request
- Make sure that there's good test coverage for the changes included in
this PR
- Run tests locally before pushing a PR (./gradlew check)
- Code should have comments where applicable. Particularly
hard-to-understand
areas deserve good in-line documentation.
- Include changes and enhancements to the documentation (in
site/content/in-dev/unreleased)
- For Work In Progress Pull Requests, please use the Draft PR feature.
Make sure to add the information BELOW this comment.
Everything in this comment will NOT be added to the PR description.
-->
--
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]
