[
https://issues.apache.org/jira/browse/HDFS-8515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14598578#comment-14598578
]
Haohui Mai commented on HDFS-8515:
----------------------------------
It might make sense to move all the code into the {o.a.h.web.http2} package.
{code}
+
+ // whether to log http2 frame for debugging
+ public static final String DFS_HTTP2_VERBOSE_KEY = "dfs.http2.verbose";
+ public static final boolean DFS_HTTP2_VERBOSE_DEFAULT = false;
+ if (verbose) {
+ frameReader =
+ new Http2InboundFrameLogger(new DefaultHttp2FrameReader(),
+ FRAME_LOGGER);
+ frameWriter =
+ new Http2OutboundFrameLogger(new DefaultHttp2FrameWriter(),
+ FRAME_LOGGER);
{code}
Instead of adding a new configuration, a better approach might be adding a
logger into {{ServerHttp2ConectionHandler}} and check whether the debug log is
enabled.
{code}
+ private static final ChannelMetadata METADATA = new ChannelMetadata(false);
+
+ private final ChannelHandlerContext http2ConnHandlerCtx;
+
+ private final Http2Stream connStream;
+
+ private final Http2Stream stream;
+
{code}
There should be no empty lines between these members.
{code}
+ private final Http2LocalFlowController localFlowController;
+
+ private final Http2RemoteFlowController remoteFlowController;
+
{code}
It might make sense to separate the flow control logic into a separate patch.
{code}
+ encoder.writeHeaders(http2ConnHandlerCtx, stream.id(),
+ (Http2Headers) msg, 0, endOfStream,
http2ConnHandlerCtx.newPromise());
{code}
encoder is not thread-safe. It seems to me the right approach is to run the
write in the event loop of the parent channel. The read path might have the
same issue.
{code}
+public class LastChunkedInput implements ChunkedInput<Object> {
+public class LastMessage {
{code}
To me both {{LastChunkedInput}} and {{LastMessage}} look like more of an
optimization right now. A simpler approach is to send an empty HEADER with the
end-of-stream bit on to tell the remote peer that the stream has been closed.
{code}
+public abstract class AbstractTestHttp2Server {
{code}
It can be a utility class instead of asking all HTTP2 test cases to inherit it.
{code}
+public class TestHttp2ServerMultiThread extends AbstractTestHttp2Server {
{code}
I'm yet to be convinced that testing of mutli threading is required right now.
Maybe having some coverage of the basic funciationlities is a higher priority.
> Abstract a DTP/2 HTTP/2 server
> ------------------------------
>
> Key: HDFS-8515
> URL: https://issues.apache.org/jira/browse/HDFS-8515
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Reporter: Duo Zhang
> Assignee: Duo Zhang
> Attachments: HDFS-8515-v1.patch, HDFS-8515-v2.patch,
> HDFS-8515-v3.patch, HDFS-8515-v4.patch, HDFS-8515.patch
>
>
> Discussed in HDFS-8471.
> https://issues.apache.org/jira/browse/HDFS-8471?focusedCommentId=14568196&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14568196
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
