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.
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
[FLINK-8768][network] Let NettyMessageDecoder inherit from LengthFieldBasedFrameDecoder #5570
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm % dead code
* </pre> | ||
*/ | ||
static class NettyMessageDecoder extends LengthFieldBasedFrameDecoder { | ||
private final boolean restoreOldNettyBehaviour; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop the unused field?
@@ -44,10 +44,11 @@ | |||
*/ | |||
public class NettyMessageSerializationTest { | |||
|
|||
public static final boolean RESTORE_OLD_NETTY_BEHAVIOUR = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dead code ;)
…dBasedFrameDecoder This replaces one additional step from the pipeline and does not only remove overhead there but also allows use to override the #extractFrame() method to restore the old Netty 4.0.27 behaviour for non-credit based code paths which had a bug with Netty >= 4.0.28 there (see FLINK-8759).
ok, done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. Thanks for the contribution @NicoK and the review @pnowojski. Merging this PR.
…dBasedFrameDecoder This replaces one additional step from the pipeline and does not only remove overhead there but also allows use to override the #extractFrame() method to restore the old Netty 4.0.27 behaviour for non-credit based code paths which had a bug with Netty >= 4.0.28 there (see FLINK-8759). This closes apache#5570.
…dBasedFrameDecoder This replaces one additional step from the pipeline and does not only remove overhead there but also allows use to override the #extractFrame() method to restore the old Netty 4.0.27 behaviour for non-credit based code paths which had a bug with Netty >= 4.0.28 there (see FLINK-8759). This closes apache#5570.
…dBasedFrameDecoder This replaces one additional step from the pipeline and does not only remove overhead there but also allows use to override the #extractFrame() method to restore the old Netty 4.0.27 behaviour for non-credit based code paths which had a bug with Netty >= 4.0.28 there (see FLINK-8759). This closes apache#5570.
…dBasedFrameDecoder This replaces one additional step from the pipeline and does not only remove overhead there but also allows use to override the #extractFrame() method to restore the old Netty 4.0.27 behaviour for non-credit based code paths which had a bug with Netty >= 4.0.28 there (see FLINK-8759). This closes apache#5570.
…dBasedFrameDecoder This replaces one additional step from the pipeline and does not only remove overhead there but also allows use to override the #extractFrame() method to restore the old Netty 4.0.27 behaviour for non-credit based code paths which had a bug with Netty >= 4.0.28 there (see FLINK-8759). This closes apache#5570.
…dBasedFrameDecoder This replaces one additional step from the pipeline and does not only remove overhead there but also allows use to override the #extractFrame() method to restore the old Netty 4.0.27 behaviour for non-credit based code paths which had a bug with Netty >= 4.0.28 there (see FLINK-8759). This closes #5570. Signed-off-by: Nico Kruber <nico@data-artisans.com>
What is the purpose of the change
Instead of being two steps in the channel pipeline,
NettyMessageDecoder
could derive fromLengthFieldBasedFrameDecoder
to reduce overhead and give us more control over the protocol.As a first step, we will use this to override the
#extractFrame()
method to restore the old Netty 4.0.27 behaviour for non-credit based code paths which had a bug with Netty >= 4.0.28 (see FLINK-8759).Brief change log
NettyMessageDecoder
inherit fromLengthFieldBasedFrameDecoder
(beware that this changes the decoder from aMessageToMessageDecoder
to aByteToMessageDecoder
with different cleanup invariants!)Verifying this change
This change is already covered by existing tests, such as
NettyMessageSerializationTest
or other network tests using the encoding/decoding pipeline.Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation