Skip to content

HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode #3023

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.

Sign up for GitHub

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

Jump to bottom
Merged
captainzmc merged 3 commits into from
Feb 15, 2022
Merged

HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode #3023

captainzmc merged 3 commits into from
Feb 15, 2022

Conversation

@guohao-rosicky
Copy link
Contributor

@guohao-rosicky guohao-rosicky commented Jan 27, 2022

What changes were proposed in this pull request?

Data Channel abstraction on datanode

What is the link to the Apache JIRA

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

How was this patch tested?

see #2860

@captainzmc captainzmc requested a review from szetszwo January 27, 2022 07:58
szetszwo
szetszwo reviewed Jan 27, 2022
View reviewed changes
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.

@guohao-rosicky , thanks for splitting #2860 . Please see the comments inlined.

.../java/org/apache/hadoop/ozone/container/common/transport/server/ratis/StreamDataChannel.java Outdated
Comment on lines 31 to 32
Copy link
Contributor

@szetszwo szetszwo Jan 27, 2022

Choose a reason for hiding this comment

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

Checked the #2860 . This link(..) method is for

    blockManager.putBlock(kvContainer, blockData);

We should call ContainerStateMachine.submitTask(..) instead of blockManager.putBlock(..). Otherwise, it will skip all the steps in-between.

So, we should remove this method and also this the interface since the interface becomes empty.

Copy link
Contributor Author

@guohao-rosicky guohao-rosicky Jan 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@szetszwo
Like https://github.com/apache/ozone/pull/2860/files#diff-731ef3fb5d0f1e895fb7e91ee065e0a6177643164f9c6287134e3529cdf98a71R161
I need to get the BCSID at the putBlock stage.

And in https://issues.apache.org/jira/browse/HDDS-6137
We will add the putBlock into stream and need to get the BCSID through the link method.
See guohao-rosicky@9ef1867#diff-d7517af5785df16a7d6507c34f75ddc93969875fa24250033aeba5886bb831feR127

Copy link
Contributor

@szetszwo szetszwo Jan 28, 2022

Choose a reason for hiding this comment

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

I need to get the BCSID at the putBlock stage.

In ContainerStateMachine.link(..), build a DispatcherContext with the BCSID and then pass the DispatcherContext to submitTask(..), which will eventually call blockManager.putBlock(..).

Copy link
Contributor Author

@guohao-rosicky guohao-rosicky Feb 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@szetszwo
I add ​putBlock into the channel, please check the logic of smallFileChannel, blockData is temporarily stored in the channel, so I need to return blockData in the channel, Is this bad for channel abstraction.

I added a comment in #2860, please see:
https://github.com/apache/ozone/pull/2860/files#r800366861
and
https://github.com/apache/ozone/pull/2860/files#r800366033

This helps us to discuss the problem.

...ice/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/StreamDataChannelBase.java Outdated
Comment on lines 61 to 63
Copy link
Contributor

@szetszwo szetszwo Jan 27, 2022

Choose a reason for hiding this comment

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

getFile() is unused. Let's add it later.

Copy link
Contributor Author

@guohao-rosicky guohao-rosicky Jan 28, 2022

Choose a reason for hiding this comment

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

yes, I'm going to delete it

@guohao-rosicky
Copy link
Contributor Author

guohao-rosicky commented Feb 10, 2022

I have removed interface, and I will modify the implementation of putBlock in #2860 later.
@szetszwo Please take a look.

szetszwo
szetszwo approved these changes Feb 10, 2022
View reviewed changes
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

captainzmc commented Feb 15, 2022

By retriggering CI, I found that the test case reporting the error is irrelevant to the current change. The CI of HDDS-4454 branch is not stable at present, we can do rebase again after the master stable.

We can merge this PR and then continue the optimization of small file and put block. Thanks @guohao-rosicky for the contribution and thanks @szetszwo for the review.

@captainzmc captainzmc merged this pull request into apache:HDDS-4454 Feb 15, 2022
@szetszwo
Copy link
Contributor

szetszwo commented Feb 15, 2022

@captainzmc , thanks for merging this.

szetszwo pushed a commit that referenced this pull request Feb 16, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (#3023
f637b1e 
)
szetszwo pushed a commit that referenced this pull request Mar 15, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (#3023
4dcc108 
)
szetszwo pushed a commit that referenced this pull request Mar 24, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (#3023
f64365b 
)
szetszwo pushed a commit to szetszwo/ozone that referenced this pull request May 6, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (ap…
b281043 
…ache#3023)
szetszwo pushed a commit that referenced this pull request May 13, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (#3023
7a6ed81 
)
szetszwo pushed a commit that referenced this pull request May 24, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (#3023
f5290d3 
)
szetszwo pushed a commit that referenced this pull request Jun 9, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (#3023
d82f075 
)
captainzmc pushed a commit to captainzmc/hadoop-ozone that referenced this pull request Jul 4, 2022
@guohao-rosicky @captainzmc
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (ap…
3cc2a7e 
…ache#3023)
szetszwo pushed a commit that referenced this pull request Oct 25, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (#3023
3dde9e2 
)

(cherry picked from commit 3cc2a7e)
(cherry picked from commit de690f6c259437bdce4389c7b06b189fa4cd20c0)
szetszwo pushed a commit that referenced this pull request Nov 7, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (#3023
4ce474c 
)

(cherry picked from commit 3cc2a7e)
(cherry picked from commit de690f6c259437bdce4389c7b06b189fa4cd20c0)
(cherry picked from commit 3dde9e2)
szetszwo pushed a commit that referenced this pull request Dec 1, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (#3023
fbb41ff 
)

(cherry picked from commit 4ce474c)
szetszwo pushed a commit that referenced this pull request Dec 16, 2022
@guohao-rosicky @szetszwo
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (#3023
8a5f2d6 
)

(cherry picked from commit 4ce474c)
nishitpatira pushed a commit to nishitpatira/ozone that referenced this pull request Dec 16, 2022
@guohao-rosicky
HDDS-6229. [Ozone-Streaming] Data Channel abstraction on datanode (ap…
8b4fca5 
…ache#3023)

(cherry picked from commit 4ce474c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@szetszwo szetszwo szetszwo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@guohao-rosicky @captainzmc @szetszwo