HDDS-15864. Branch merge checklist for Short Circuit read#503
Conversation
Gargi-jais11
left a comment
Gargi-jais11
left a comment
There was a problem hiding this comment.
Thanks @ChenSammi for preparing this. LGTM!
|
|
||
| ## 2. Documentation | ||
|
|
||
| [User Documentation](https://github.com/apache/ozone/blob/HDDS-10685/hadoop-hdds/docs/content/feature/Short-Circuit-Read.md) of Short Circuit Read has been added. |
There was a problem hiding this comment.
@ChenSammi @Gargi-jais11
This PR completely removes the old website and Hugo since it is now unused. So I suggest to use https://ozone.apache.org/docs/next/administrator-guide/configuration/performance/short-circuit-local-reads
There was a problem hiding this comment.
Thanks @ChenSammi for the working on the checklist. Please check the inline comment. Also we may need to create followup Jira to fix urls that pointing to feature branch code to master branch after merge.
I plan to merge this patch after feature branch is merged, so URL can be updated later. |
priyeshkaratha
left a comment
priyeshkaratha
left a comment
There was a problem hiding this comment.
Thanks @ChenSammi for clarifying comments. Changes LGTM
errose28
left a comment
errose28
left a comment
There was a problem hiding this comment.
Thanks for adding this. Left some comments inline.
|
|
||
| ## 3. Design, attached the docs | ||
|
|
||
| Design document can be found here : [Short Circuit Read Support](https://github.com/apache/ozone/blob/HDDS-10685/hadoop-hdds/docs/content/design/short-circuit-read.md). |
There was a problem hiding this comment.
Can you convert the pdf to markdown and re-commit it to the branch? This is better for AI integration.
|
|
||
| ## 9. Possible incompatible changes/used feature flag | ||
|
|
||
| Short-circuit read is gated by the non-rolling upgrade framework as HDDSLayoutFeature.SHORT_CIRCUIT_READS (layout version 11). |
There was a problem hiding this comment.
Why is the layout feature necessary? It looks like this just creates a socket and closes it on shutdown. How would this break on downgrade?
Also it looks like new clients can request short circuit read from an old server it will be ignored. This seems fine since the field is phrased as "request" but that is worth noting in this section too.
|
|
||
| End-to-end read performance is therefore dominated by local disk I/O, similar to a direct file read, rather than by gRPC data transfer. | ||
|
|
||
| A benchmark was run against feature branch HDDS-10685, and get this result https://docs.google.com/document/d/1xmjCbK4rP287pPN0_iOonnQee6HgwSI3h5hVbtgfu3k/edit?tab=t.0. |
There was a problem hiding this comment.
This doc isn't very large, we can put its content directly in this file. We should not commit links to google docs which can disappear or lose access to git.
|
|
||
| Short-circuit communication uses a UNIX domain socket (`ozone.domain.socket.path`) between the client and Datanode. | ||
|
|
||
| It follows the same [Hadoop Socket Path Security](https://cwiki.apache.org/confluence/spaces/HADOOP2/pages/120730260/SocketPathSecurity) rules as HDFS short-circuit reads. |
There was a problem hiding this comment.
agreed, as a TODO item we should copy this to our doc site.
|
|
||
| ## 2. Documentation | ||
|
|
||
| [User Documentation](https://github.com/apache/ozone/blob/HDDS-10685/hadoop-hdds/docs/content/feature/Short-Circuit-Read.md) of Short Circuit Read has been added. |
There was a problem hiding this comment.
the feature would require topology awareness to maximize locality https://ozone.apache.org/docs/next/administrator-guide/configuration/performance/topology/#optimizing-read-paths
let's update user doc post merge.
5 participants
What changes were proposed in this pull request?
Add branch merge checklist for short-circuit read.
What is the link to the Apache Jira?
HDDS-15864
How was this patch tested?