-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[BUG]Fix broker read buffer size from input stream #3881
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
Conversation
| "end of file reached"); | ||
| } | ||
| return ByteBuffer.wrap(buf, 0, readLength); | ||
| logger.info("read length:" + length + ", readBufferSize:" + readBufferSize + ", return length:" + readLength); |
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.
Maybe too many logs here.
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.
Turn log level to debug.
yiguolei
left a comment
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.
some comments
| "end of file reached"); | ||
| } | ||
| return ByteBuffer.wrap(buf, 0, readLength); | ||
| logger.debug("read buffer from input stream, buffer size:" + buf.capacity() + ", read length:" + readLength); |
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.
if (logger.isDebugEnable()) {
logger.debug("read buffer from input stream, buffer size: {}, read length: {}", buf.capacity(), readLength);
}
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.
done
morningman
left a comment
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
This bug is introduced by PR #3784 In #3784, I remove the `Catalog.getInstance()`, and use `Catalog.getCurrentCatalog()` instead. But actually, there are some place that should use the serving catalog explicitly. Mainly changed: 1. Add a new method `getServingCatalog()` to explicitly return the real catalog instance. 2. Fix a compile bug of broker introduced by #3881
This bug is introduced by PR apache#3784 In apache#3784, I remove the `Catalog.getInstance()`, and use `Catalog.getCurrentCatalog()` instead. But actually, there are some place that should use the serving catalog explicitly. Mainly changed: 1. Add a new method `getServingCatalog()` to explicitly return the real catalog instance. 2. Fix a compile bug of broker introduced by apache#3881
This bug is introduced by PR apache#3784 In apache#3784, I remove the `Catalog.getInstance()`, and use `Catalog.getCurrentCatalog()` instead. But actually, there are some place that should use the serving catalog explicitly. Mainly changed: 1. Add a new method `getServingCatalog()` to explicitly return the real catalog instance. 2. Fix a compile bug of broker introduced by apache#3881
Fix #3879
This commit fixs a bug that broker cannot read the full length of buffer size, when the buffer size is set larger than 128k.
This bug will cause the data size returned by pread request to be less than 128K all the time.