[#3806] Setting WRITE_BUFFER_LOW_WATER_MARK before WRITE_BUFFER_HIGH_…#5060
Closed
normanmaurer wants to merge 1 commit into
Closed
[#3806] Setting WRITE_BUFFER_LOW_WATER_MARK before WRITE_BUFFER_HIGH_…#5060normanmaurer wants to merge 1 commit into
normanmaurer wants to merge 1 commit into
Conversation
Member
Author
Member
Author
|
Also /cc @rkapsi |
| */ | ||
| @Deprecated | ||
| public static final ChannelOption<Integer> WRITE_BUFFER_LOW_WATER_MARK = valueOf("WRITE_BUFFER_LOW_WATER_MARK"); | ||
| public static final ChannelOption<Integer> WRITE_BUFFER_WATER_MARK = valueOf("WRITE_BUFFER_WATER_MARK"); |
Member
There was a problem hiding this comment.
Shouldn't it be ChannelOption<WriteBufferWaterMark>?
Contributor
There was a problem hiding this comment.
@normanmaurer second rkapsi, it is my mistake for setting to integer?
Member
Author
There was a problem hiding this comment.
yes it should ... let me fix it
Member
|
LGTM and thanks for fixing this long forgotten ticket. |
normanmaurer
force-pushed
the
write_buffer
branch
2 times, most recently
from
ee877e0 to
0883688
Compare
| AUTOREAD_UPDATER = autoReadUpdater; | ||
|
|
||
| AtomicReferenceFieldUpdater<DefaultChannelConfig, WriteBufferWaterMark> watermarkUpdater = | ||
| PlatformDependent.newAtomicReferenceFieldUpdater(DefaultChannelConfig.class, "writeBufferWaterMark"); |
There was a problem hiding this comment.
Define a constant instead of duplicating this literal "writeBufferWaterMark" 3 times. 
Member
|
ship it |
| * Returns the {@link WriteBufferWaterMark} which is used for setting the high and low | ||
| * water mark of the write buffer. | ||
| */ | ||
| WriteBufferWaterMark getWriteBufferWaterMark(); |
Member
There was a problem hiding this comment.
Could you put the getter before its corresponding setter like other access methods?
Member
|
LGTM sans a few nits |
…WATER_MARK results in an internal Exception Motivation: Setting the WRITE_BUFFER_LOW_WATER_MARK before WRITE_BUFFER_HIGH_WATER_MARK results in an internal Exception (appears only in the logs) if the value is larger than the default high water mark value. The WRITE_BUFFER_HIGH_WATER_MARK call appears to have no effect in this context. Setting the values in the reverse order works. Modifications: - deprecated ChannelOption.WRITE_BUFFER_HIGH_WATER_MARK and ChannelOption.WRITE_BUFFER_LOW_WATER_MARK. - add one new option called ChannelOption.WRITE_BUFFER_WATER_MARK. Result: The high/low water mark values limits caused by default values are removed. Setting the WRITE_BUFFER_LOW_WATER_MARK before WRITE_BUFFER_HIGH_WATER_MARK results in an internal Exception (appears only in the logs) if the value is larger than the default high water mark value. The WRITE_BUFFER_HIGH_WATER_MARK call appears to have no effect in this context. Setting the values in the reverse order works.
normanmaurer
force-pushed
the
write_buffer
branch
from
0883688 to
ec826c8
Compare
Member
Author
|
Cherry-picked into 4.1 as 3e5dcb5. @jiafu1115 thanks again! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…WATER_MARK results in an internal Exception
Motivation:
Setting the WRITE_BUFFER_LOW_WATER_MARK before WRITE_BUFFER_HIGH_WATER_MARK results in an internal Exception (appears only in the logs) if the value is larger than the default high water mark value. The WRITE_BUFFER_HIGH_WATER_MARK call appears to have no effect in this context.
Setting the values in the reverse order works.
Modifications:
ChannelOption.WRITE_BUFFER_LOW_WATER_MARK.
Result:
The high/low water mark values limits caused by default values are removed.
Setting the WRITE_BUFFER_LOW_WATER_MARK before WRITE_BUFFER_HIGH_WATER_MARK results in an internal Exception (appears only in the logs) if the value is larger than the default high water mark value. The WRITE_BUFFER_HIGH_WATER_MARK call appears to have no effect in this context.
Setting the values in the reverse order works.