HDDS-1753. Datanode unable to find chunk while replication data using ratis.#1318
Merged
lokeshj1703 merged 3 commits intoapache:trunkfrom Aug 28, 2019
Merged
HDDS-1753. Datanode unable to find chunk while replication data using ratis.#1318lokeshj1703 merged 3 commits intoapache:trunkfrom
lokeshj1703 merged 3 commits intoapache:trunkfrom
Conversation
bshashikant
changed the title
|
💔 -1 overall
This message was automatically generated. |
lokeshj1703
requested changes
Aug 21, 2019
Contributor
There was a problem hiding this comment.
@bshashikant Thanks for working on this! The changes look good to me. I have couple of minor comments. There are a few checkstyle issues and test failures seem related.
Contributor
There was a problem hiding this comment.
We can directly use the getServer().getGroupInfo(..) api here and do not need the while loop.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
lokeshj1703
requested changes
Aug 27, 2019
Contributor
lokeshj1703
left a comment
lokeshj1703
left a comment
There was a problem hiding this comment.
There are a few checkstyle issues. TestBlockDeletion failure seems related.
| XceiverServerRatis ratisServer = | ||
| (XceiverServerRatis) ozoneContainer.getWriteChannel(); | ||
| long minReplicatedIndex = ratisServer.getMinReplicatedIndex(PipelineID | ||
| .valueOf(UUID.fromString(containerData.getOriginPipelineId()))); |
Contributor
There was a problem hiding this comment.
If the pipeline does not exist, it should throw GroupMismatchException here. We can check what exception is thrown once. In that case we need to return true.
Contributor
Author
There was a problem hiding this comment.
Addressed in the latest patch.
| Mockito.when(ozoneContainer.getContainerSet()) | ||
| .thenReturn(containerSet); | ||
| Mockito.when(ozoneContainer.getWriteChannel()) | ||
| .thenReturn(null); |
Contributor
There was a problem hiding this comment.
Since it is used in all the test classes, we can add the logic for mocking ozoneContainer in a TestUtil class.
Contributor
Author
There was a problem hiding this comment.
Addressed in the latest patch.
| cluster.shutdownHddsDatanode(follower.getDatanodeDetails()); | ||
| key.write(testData); | ||
| key.close(); | ||
| XceiverClientSpi xceiverClient = |
Contributor
There was a problem hiding this comment.
Can we please add a comment here that the container will be closed? Similarly a comment regarding deleting the key just so that we can separate the various sections of the test.
Contributor
Author
There was a problem hiding this comment.
addressed in the latest patch..
Contributor
|
@bshashikant Thanks for updating the PR! The changes look good to me. +1. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
|
@bshashikant Thanks for working on the PR! I have merged it with trunk. |
amahussein
pushed a commit
to amahussein/hadoop
that referenced
this pull request
Oct 29, 2019
RogPodge
pushed a commit
to RogPodge/hadoop
that referenced
this pull request
Mar 25, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.