Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use equal() instead of identity in readonly TNode #24

Merged
merged 1 commit into from
Aug 21, 2019
Merged

Use equal() instead of identity in readonly TNode #24

merged 1 commit into from
Aug 21, 2019

Conversation

rovarga
Copy link
Contributor

@rovarga rovarga commented Nov 19, 2016

Instead of requiring a match on identity, use equal()
to compare the stored and lookup keys. This fixes
https://bugs.opendaylight.org/show_bug.cgi?id=6577.

Signed-off-by: Robert Varga nite@hq.sk

rovarga added a commit to rovarga/scala that referenced this pull request Nov 19, 2016
https://bugs.opendaylight.org/show_bug.cgi?id=6577 has uncovered
an issue with a Java port of this implementation.

OpenDaylight uses read-only snapshots to implement MVCC behavior
and has the usage pattern of having multiple concurrent readers
and a single writer.

Under load with about 100K items in the map some 6 items fail
to be found when we look at the read-only snapshot of state
before deletion and look for items we know to have deleted.

Additional investigation has shown that the items are present
in the map with a non-identical, but equal key. Using entrySet()
to locate the stored key and using that key for lookup results
in the value being returned -- which is a violation of Map
contract.

This patch is a backport of
romix/java-concurrent-hash-trie-map#24,
which fixes the issue in the Java port.

Signed-off-by: Robert Varga <nite@hq.sk>
Instead of requiring a match on identity, use equal()
to compare the stored and lookup keys. This fixes
https://bugs.opendaylight.org/show_bug.cgi?id=6577.

This is a real bug coming from difference in operators
between Java and Scala. Scala == boils down to .equals(),
whereas in Java it is an identity check (eq in Scala).

Signed-off-by: Robert Varga <nite@hq.sk>
@rovarga
Copy link
Contributor Author

rovarga commented Nov 19, 2016

@romix would you be so kind as to spin a new release with just this PR?

@romix
Copy link
Owner

romix commented Aug 21, 2019

Thanks for the fix!

@romix romix merged commit 5cf056a into romix:master Aug 21, 2019
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.

None yet

2 participants