Skip to content

Conversation

@captainzmc
Copy link
Member

What changes were proposed in this pull request?

Streaming supports writing in Pipline mode

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-5486

@captainzmc captainzmc requested a review from szetszwo September 27, 2021 12:10
@szetszwo szetszwo changed the title HDDS-5705. [Ozone-Streaming] Streaming supports writing in Pipline mode HDDS-5486. [Ozone-Streaming] Streaming supports writing in Pipline mode Sep 28, 2021
@captainzmc
Copy link
Member Author

captainzmc commented Sep 28, 2021

@szetszwo Please ignore the problem I asked earlier, I have found the root cause.

@szetszwo
Copy link
Contributor

... I have found the root cause.

That's great!

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

A minor comment inlined.

Copy link
Contributor

@szetszwo szetszwo Sep 28, 2021

Choose a reason for hiding this comment

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

RoutingTable only needs RaftPeerId. So we can have List<RaftPeerId> instead of List<RaftPeer>.

@bshashikant
Copy link
Contributor

@captainzmc , can you add some description on the jira for this? The patch otherwise looks good. Please remove the commented out code.

@captainzmc
Copy link
Member Author

@captainzmc , can you add some description on the jira for this? The patch otherwise looks good. Please remove the commented out code.

Yes, I have updated jIRA and add the description.

@captainzmc
Copy link
Member Author

hi @szetszwo, The code has been updated and tested. Can you help take another look?

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

Thanks for the update. Some comments inlined.


public RoutingTable getRoutingTable(Pipeline pipeline) {
RaftPeerId primary = null;
List<RaftPeerId> raftPeers = new ArrayList();
Copy link
Contributor

Choose a reason for hiding this comment

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

It should have an "<>", i.e. new ArrayList<>().

Comment on lines 199 to 209
RaftPeer.Builder raftPeerBuilder = RaftPeer.newBuilder();
raftPeerBuilder.setId(dn.getUuidString()).setAddress(dn.getIpAddress()
+ ":" + dn.getPort(DatanodeDetails.Port.Name.RATIS_SERVER)
.getValue());
raftPeerBuilder.setDataStreamAddress(dn.getIpAddress()
+ ":" + dn.getPort(DatanodeDetails.Port.Name.RATIS_DATASTREAM)
.getValue());
raftPeerBuilder.setAdminAddress(dn.getIpAddress()
+ ":" + dn.getPort(DatanodeDetails.Port.Name.RATIS_ADMIN).getValue());
raftPeerBuilder.setClientAddress(dn.getIpAddress()
+ ":" + dn.getPort(DatanodeDetails.Port.Name.RATIS).getValue());
Copy link
Contributor

Choose a reason for hiding this comment

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

Since RaftPeer is not needed, let's just create a RaftPeerId.

      final RaftPeerId id = RaftPeerId.valueOf(dn.getUuidString());


}
} catch (IOException e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

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

It should return null when there is an exception.

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

+1 the change looks good.

@captainzmc
Copy link
Member Author

CI is performing well on my personal branch. This failure should be accidental.

@szetszwo szetszwo merged this pull request into apache:HDDS-4454 Sep 30, 2021
szetszwo pushed a commit to szetszwo/ozone that referenced this pull request May 6, 2022
captainzmc added a commit to captainzmc/hadoop-ozone that referenced this pull request Jul 4, 2022
szetszwo pushed a commit that referenced this pull request Oct 25, 2022
…de (#2682)

(cherry picked from commit 2e85eeb)
(cherry picked from commit 10b14300d274cf224f56763c66dea9c2d8a6f731)
szetszwo pushed a commit that referenced this pull request Nov 7, 2022
…de (#2682)

(cherry picked from commit 2e85eeb)
(cherry picked from commit 10b14300d274cf224f56763c66dea9c2d8a6f731)
(cherry picked from commit 2791680)
szetszwo pushed a commit that referenced this pull request Dec 1, 2022
szetszwo pushed a commit that referenced this pull request Dec 16, 2022
nishitpatira pushed a commit to nishitpatira/ozone that referenced this pull request Dec 16, 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