netty: (Nio|Epoll)EventLoop.pendingTasks should not block when called outside of event loop

Expected behavior

EventLoop.pendingTasks should be (reasonably) cheap to invoke so it can be used within observability.

Actual behavior

We’ve been experimenting with overriding EventExecutorChooserFactory with more superior load balancing algorithms and found out that it might be quite non-trivial to support pending-tasks load metric (pick the least loaded event loop).

After poking around the code, it seems when querying pendingTasks of either NioEventLoop or EpollEventLoop, the method blocks if called outside of said event loop. Put it this way, it’s only cheap to query event loop’s pending tasks from within the same event loop. Perhaps I miss some (historical) context for pendingTask, but for an uneducated reader, it just defeats the purpose of a method providing visibility into an event loop.

I was hoping to pick up some context on the current behavior of pendingTasks and see if we ever want/will to change it.

About this issue

Original URL
State: closed
Created 6 years ago
Comments: 24 (24 by maintainers)

Most upvoted comments

@johnou I apologize if I sounded impatient 😃 I was reading your question from the GitHub email. Here’s what I got:

@nitsanw is it safe to invoke MpscQueue.size() from multiple threads?

It appears that further edits do not get sent as further e-mails.

So, yes, it’s safe to call size on any queue in jctools from any thread, no issue. Furthermore, current queue being used is either the MpscChunkedArrayQueue or MpscUnboundedArrayQueue: https://github.com/netty/netty/blob/06f3574e46e0623a45a93d36dbe7aa3d7455a995/common/src/main/java/io/netty/util/internal/PlatformDependent.java#L823

Both of which have a very fast size() method.

As a side note, I’m not sure why in the non-unsafe case you do not use MpscChunkedAtomicArrayQueue: https://github.com/netty/netty/blob/06f3574e46e0623a45a93d36dbe7aa3d7455a995/common/src/main/java/io/netty/util/internal/PlatformDependent.java#L859

Thanks

nitsanw on Aug 16, 2018

@nitsanw and also asked if it was unsafe and posed risks ie. livelock, but given your comment now I will assume it’s safe to invoke size() from multiple threads, thanks!

johnou on Aug 16, 2018

You asked if it was safe, right?

On 16 Aug 2018, at 16:58, Johno Crawford notifications@github.com wrote:

@nitsanw was the yes for safe or risk producing a livelock? 😃

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

nitsanw on Aug 16, 2018

@johnou I’m pretty sure the livelock issue wasn’t inherently related to the single-consumerness. Looking at the history it appeared to be more likely linked to the custom modifications made (e.g. possibly RecyclableMpscLinkedQueueNode).

Of course the reported size may not be “precise” due to the lazy semantics but should hopefully be “safe”.

njhill on Aug 16, 2018

@vkostyukov that would be awesome… Like you already stated we should just make this “as cheap as possible”.

normanmaurer on Aug 14, 2018