[
https://issues.apache.org/jira/browse/OAK-6051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15964180#comment-15964180
]
Michael Dürig commented on OAK-6051:
------------------------------------
I agree, we should not change this but keep relying on {{fastEquals()}} instead:
* Switching to {{equals()}} defeats the purpose as we want to be fast here and
don't care about false negatives.
* Switching to {{equals()}} introduces a regressions risk as the failing tests
above show. (More on this in a later comment).
However we should clarify a couple of things:
* Add Javadoc to {{SegmentNodeState#fastEquals()}} clearly stating that a
return value of {{true}} means equal while a return value of {{false}} not
necessarily means not equal. It should also state that the "false negatives"
are an implementation details and callers cannot rely on them being stable.
* {{Commit.hasChanges()}} has similar semantics as {{fastEquals()}} and the
Javadoc should reflect that.
> Clarify migration tests failures when switching Commit#hasChanges
> implementations
> ---------------------------------------------------------------------------------
>
> Key: OAK-6051
> URL: https://issues.apache.org/jira/browse/OAK-6051
> Project: Jackrabbit Oak
> Issue Type: Task
> Components: segment-tar
> Reporter: Andrei Dulceanu
> Assignee: Andrei Dulceanu
> Priority: Minor
> Fix For: 1.8, 1.7.3
>
>
> Currently, the implementation used to tell if there are changes in a commit
> to be merged (comparing the base state against the node state) uses
> {{SegmentNodeState#fastEquals}} directly (and consequently making that method
> public). Although comparing the node states through {{equals}}, which in turn
> should then delegate to {{fastEquals}}, should work, the following tests are
> failing when using this method:
> {code}
> Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.41 sec <<<
> FAILURE! - in
> org.apache.jackrabbit.oak.segment.migration.ExternalToExternalMigrationTest
> blobsExistsOnTheNewBlobStore(org.apache.jackrabbit.oak.segment.migration.ExternalToExternalMigrationTest)
> Time elapsed: 0.082 sec <<< ERROR!
> java.io.IOException: java.lang.NullPointerException
> Caused by: java.lang.NullPointerException
> Running
> org.apache.jackrabbit.oak.segment.migration.SegmentToExternalMigrationTest
> Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.163 sec <<<
> FAILURE! - in
> org.apache.jackrabbit.oak.segment.migration.SegmentToExternalMigrationTest
> blobsExistsOnTheNewBlobStore(org.apache.jackrabbit.oak.segment.migration.SegmentToExternalMigrationTest)
> Time elapsed: 0.062 sec <<< ERROR!
> java.io.IOException: java.lang.NullPointerException
> Caused by: java.lang.NullPointerException
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
