Skip to content
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

Update statistics for org.jboss.netty.channel.FileRegion writes #329

Closed

Conversation

whiter4bbit
Copy link
Contributor

Handle FileRegion writes in ChannelStatsHandler

Problem

Currently, when writing instance of FileRegion (in custom Codec for example), handler logs many annoying messages:

WAR [20141220-05:51:59.151] channel: ChannelStatsHandler received non-channelbuffer write

And sent bytes count from FileRegion not included in sent_bytes counter.

Solution

Check if message is instance of FileRegion and increment stats counters by FileRegion.getCount value

Result

Sent bytes from FileRegion are included in sent_bytes and connection_sent_bytes counters

@mosesn
Copy link
Contributor

mosesn commented Dec 23, 2014

Very cool! Have you been doing any zero-copy stuff with finagle? I'd be very curious if you're able to share. Could you also update the "messageReceived" method to support FileRegion?

@whiter4bbit
Copy link
Contributor Author

yes, I think, I can share example bit later, regarding messageReceived - this method invoked during upstream handling (for incoming message) and can never be FileRegion, sinceFileRegion can just transfer data to WritableByteChannel, but not read from 'readable' one.

@mosesn
Copy link
Contributor

mosesn commented Dec 29, 2014

@trustin could you weigh in? I don't think I understand netty well enough to review this.

@whiter4bbit I thought part of the idea was that if you received a request, you could zero-copy that request elsewhere. It sounds like there's something I'm missing, is there some documentation or code I should read? I tried googling for information about netty + FileRegion, but didn't come up with anything that could help me understand it.

@trustin
Copy link
Contributor

trustin commented Jan 20, 2015

This pull request itself looks good to me.

However, I think it's not a good idea to increase the counters at writeRequested(), because it only tells us when a write operation has been requested, not when the write operation has been actually fulfilled.

I would recommend overriding SimpleChannelUpstreamHandler.writeComplete() and get the number of written bytes via WriteCompletionEvent.getWrittenAmount().

@mosesn
Copy link
Contributor

mosesn commented Feb 4, 2015

@whiter4bbit I just wanted to make sure this PR didn't get lost between the cracks.

@whiter4bbit
Copy link
Contributor Author

was out of time, I will try to 're-implement things in way that @trustin proposed

@mosesn
Copy link
Contributor

mosesn commented Feb 4, 2015

Thanks!

@whiter4bbit
Copy link
Contributor Author

Created new PR: #355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants