Skip to content

OAK-9850: ConcurrentPrefetchAndUpdateIT.cacheConsistency fails occasi…#646

Merged
mreutegg merged 4 commits intoapache:trunkfrom
mreutegg:OAK-9850-3
Aug 8, 2022
Merged

OAK-9850: ConcurrentPrefetchAndUpdateIT.cacheConsistency fails occasi…#646
mreutegg merged 4 commits intoapache:trunkfrom
mreutegg:OAK-9850-3

Conversation

@mreutegg
Copy link
Copy Markdown
Contributor

@mreutegg mreutegg commented Aug 2, 2022

…onally

Keys collection passed to registerTracker must not be modified later.
NodeDocumentCache.putNonConflictingDocs() must always notify other trackers.
Enable test again.

…onally

Keys collection passed to registerTracker must not be modified later.
NodeDocumentCache.putNonConflictingDocs() must always notify other trackers.
Enable test again.
@mreutegg mreutegg requested a review from stefan-egli August 2, 2022 12:41
Copy link
Copy Markdown
Contributor

@stefan-egli stefan-egli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides fixing the test, and besides it being related to a new feature being involved (prefetch), would there also have been a possibility for outdated cache state previously?

@mreutegg
Copy link
Copy Markdown
Contributor Author

mreutegg commented Aug 2, 2022

Besides fixing the test

The test was not broken and this PR does not make any changes to the test except enable it again.

would there also have been a possibility for outdated cache state previously?

I think that's possible. The added else clause in NodeDocumentCache.putNonConflictingDocs() is not specific to prefetching.

@stefan-egli
Copy link
Copy Markdown
Contributor

Wsa wondering if it would make sense to try and come up with a test case that would have failed without this fix. Maybe it could help finding or explaining other issues (like some stray OakMerge0004 conflicts..).

@mreutegg
Copy link
Copy Markdown
Contributor Author

mreutegg commented Aug 4, 2022

Was wondering if it would make sense to try and come up with a test case that would have failed without this fix.

Running ConcurrentPrefetchAndUpdateIT in a loop eventually failed on my machine without the fix. But I agree, it would be better if there was a deterministic test that fails immediately without the fix.

…onally

Add comment about why a new HashSet is passed to registerTracker()
@mreutegg
Copy link
Copy Markdown
Contributor Author

mreutegg commented Aug 4, 2022

Test failure in oak-blob-plugins seems unrelated. I can reproduce the failure locally and created https://issues.apache.org/jira/browse/OAK-9879

…onally

Add a simple test that illustrates why changes to NodeDocumentCache are necessary
@mreutegg
Copy link
Copy Markdown
Contributor Author

mreutegg commented Aug 5, 2022

a test case that would have failed without this fix

@stefan-egli there is now a simple test that illustrates why changes to NodeDocumentCache are necessary. NodeDocumentCacheTest fails deterministically without changes to NodeDocumentCache.putNonConflictingDocs().

Copy link
Copy Markdown
Contributor

@stefan-egli stefan-egli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, nice test

@mreutegg mreutegg merged commit cb009e2 into apache:trunk Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants