[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2018-02-28 Thread NicoK
Github user NicoK commented on the issue:

https://github.com/apache/flink/pull/4529
  
let me close this PR since this will be done by upgrading netty instead


---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2018-02-27 Thread zhijiangW
Github user zhijiangW commented on the issue:

https://github.com/apache/flink/pull/4529
  
Thanks for telling me the plan of it. I will do that if necessary. :)


---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2018-02-27 Thread NicoK
Github user NicoK commented on the issue:

https://github.com/apache/flink/pull/4529
  
@zhijiangW looks like we're not upgrading the netty version in Flink 1.5 at 
this stage due to the timing, i.e. too quite late in the development cycle. We 
will do the upgrade after releasing Flink 1.5 so that we have enough time to 
test it.

If you want to try out whether an upgrade changes anything for you, please 
feel free to pull from #5571 and upgrade `flink-shaded-netty` in the root 
`pom.xml` to `4.0.56.Final-3.0` once 
[flink-shaded](https://github.com/apache/flink-shaded) 3.0 is released.


---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2018-02-23 Thread NicoK
Github user NicoK commented on the issue:

https://github.com/apache/flink/pull/4529
  
With the recent update of the netty version in flink-shaded (see 
https://github.com/apache/flink-shaded/commit/1233f1bb0e2b9fafa4260603aa130b7eb9995a7a),
 most of this PR is indirectly included. However, I probably need to fall back 
to the original code creating a copy at least for the fallback non-credit based 
paths due to the original problem.

Additionally, since the introduction of the credit-based flow control we 
should not use too much memory on the receiver side anymore: memory size in the 
range of once in our buffers, at most one (or two depending on the above) more 
copies in netty temporarily.

Ultimately, we could go one step further of decoding directly to our 
buffers here, but that is not covered by this PR and will only work with 
credit-based flow control.


---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2018-02-23 Thread zhijiangW
Github user zhijiangW commented on the issue:

https://github.com/apache/flink/pull/4529
  
Hey @NicoK , would this PR be covered in FLINK-1.5? We experienced the 
netty direct memory out of memory sometimes in production cased by 
`extractFrame`, so we expect this improvement. :)


---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2017-09-27 Thread NicoK
Github user NicoK commented on the issue:

https://github.com/apache/flink/pull/4529
  
I haven't had time to test this yet - I doubt that this is simply solved by 
using our memory segments but I need to understand the problem better.

Maybe this also goes away with [flow 
control](https://issues.apache.org/jira/browse/FLINK-7282) being added since 
then we have more control over what the sender sends and can make sure that we 
have enough buffers to process its data.


---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2017-09-06 Thread KurtYoung
Github user KurtYoung commented on the issue:

https://github.com/apache/flink/pull/4529
  
@zentol So the rationale behind this is when all netty's memory comes from 
flink's managed memory, then this is not an issue, right?



---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2017-09-06 Thread zentol
Github user zentol commented on the issue:

https://github.com/apache/flink/pull/4529
  
@KurtYoung I think 
https://github.com/apache/flink/pull/4529#issuecomment-321851427 addresses that.


---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2017-09-06 Thread KurtYoung
Github user KurtYoung commented on the issue:

https://github.com/apache/flink/pull/4529
  
If I remember correctly, the reason we chose to do an extra copy inside 
`extractFrame` is that we faced some memory problem when using `slice`. Is this 
no longer an issue or have taken care by another PR?


---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2017-08-25 Thread NicoK
Github user NicoK commented on the issue:

https://github.com/apache/flink/pull/4529
  
Thanks, @greghogan - I put most of them under FLINK-7315 now (at least the 
ones which are loosely related).

Regarding the PR itself, I still need to investigate whether the previous 
issues went away.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2017-08-15 Thread greghogan
Github user greghogan commented on the issue:

https://github.com/apache/flink/pull/4529
  
@NicoK what do you think of creating a parent issue for your collection of 
network improvements?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2017-08-14 Thread NicoK
Github user NicoK commented on the issue:

https://github.com/apache/flink/pull/4529
  
we also need to double-check this against the previous issue of 
d92e422ec7089376583a8f57043274d236c340a4 which may be solved by the way I am 
using the `LengthFieldBasedFrameDecoder` now compared to back then, or the 
changes that happened since then


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink issue #4529: [FLINK-7428][network] avoid buffer copies when receiving ...

2017-08-11 Thread NicoK
Github user NicoK commented on the issue:

https://github.com/apache/flink/pull/4529
  
FYI:
- newer netty 4.0.xx versions also use `buffer.slice(index, length)` inside 
their `extractFrame()` method (see 
https://github.com/netty/netty/commit/891be30a28c6dc5a1edf1cb5a3690644cf4ff66e) 
and despite the comment on `extractFrame()`, this is also safe when used 
outside the `decode()` method as long as the buffer is retained (and then 
released later). By letting `NettyMessageDecoder` inherit from 
`LengthFieldBasedFrameDecoder` though, we also gain a smaller channel handler 
pipeline.
- netty 4.1 offers an even improved `buffer.retainedSlice(index, length)` 
implementation (see 
https://github.com/netty/netty/commit/3a9f47216143082bdfba62e8940160856767d672) 
but we can't go that way yet due to our netty-router dependency


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---