IGNITE-20688 Fixed broken handles after binary object detached. #11272
Conversation
petrov-mg
force-pushed
the
IGNITE-20688
branch
2 times, most recently
from
958161e
to
58a1649
Compare
| @@ -179,6 +181,9 @@ | |||
|
|
|||
| BinaryMetadataMoveLegacyFolderTest.class, | |||
| BinaryContextPredefinedTypesTest.class, | |||
|
|
|||
| RawBytesObjectReaderTest.class, | |||
| CrossObjetReferenceSerializationTest.class | |||
There was a problem hiding this comment.
Let's add comma at the end of the line.
|
|
||
| U.arrayCopy(arr, start, arr0, 0, len); | ||
| CrossObjectReferenceDetector detector = new CrossObjectReferenceDetector(BinaryHeapInputStream.create(arr, start)); |
There was a problem hiding this comment.
Can we use information about handles here as an additional hint?
For example:
- Pass to this (or new one) method some parameter like
checkObjectReferences - Only if this parameter is true analyze array by detector.
- Pass this parameter as false if handles is empty (if there are no handles, there can't be references to this handle)
In this case we can also revert changes to BinaryReaderExImpl.readObjectDetached, since, for root binary object, handles will always be empty.
| } | ||
|
|
||
| /** */ | ||
| public void readObject(BinaryOutputStream out) { |
There was a problem hiding this comment.
It's not a good name for this method (and following methods with BinaryOutputStream out parameter), perhaps something like copyObject is better.
|
|
||
| if (isCrossObjRefDetected) { | ||
| try (BinaryOutputStream out = new BinaryHeapOutputStream(2 * len)) { | ||
| CrossObjectReferenceResolver resolver = new CrossObjectReferenceResolver(BinaryHeapInputStream.create(arr, start), out); |
There was a problem hiding this comment.
Reuse the same BinaryHeapInputStream as for CrossObjectReferenceDetector?
| import org.apache.ignite.internal.binary.streams.BinaryOutputStream; | ||
|
|
||
| /** */ | ||
| public class RawBytesObjectReader implements BinaryPositionReadable { |
There was a problem hiding this comment.
...BinaryObjectReader?
| /** */ | ||
| @RunWith(Parameterized.class) | ||
| @WithSystemProperty(key = IGNITE_BINARY_SORT_OBJECT_FIELDS, value = "true") | ||
| public class CrossObjetReferenceSerializationTest extends GridCommonAbstractTest { |
There was a problem hiding this comment.
Typo: CrossObjectReferenceSerializationTest
|
|
||
| while (reader.position() < objDataEndPos) { | ||
| if (findInNextObject()) { | ||
| isFound = true; |
There was a problem hiding this comment.
Looks like is safe to return true; here (we don't rely on reader.position anywhere after returning true by this method)
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
The Contribution Checklist
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summarywhereXXXX- number of JIRA issue.(see the Maintainers list)
the
green visaattached to the JIRA ticket (see TC.Bot: Check PR)Notes
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.