NIO - handling read events best practice?

[farseer]

Looking for some "best practice" advice.
The problem i am trying to solve is the following:

I have a server that waits for OP_ACCEPT and when that happens,
registers OP_READ for that channel).
When an OP_READ event is fired, my handling routine creates a "client"
object (which contains the Selection Key as a member) and add this to a
synchronized "queue". I have a number of threads blocking on that
queue when it's empty...so that when something is added, one of the
threads picks it up and processes it (reads from the socket).

However, once an OP_READ is fired, it continues to be fired until a
read occurs from that socket channel i believe. This means that if a
thread happens to not read from the queue'd client i just added, that
event is fired again, and in this case, i could have more than one
object added to my queue for the same event (i.e. if a client socket is
sending to my server, when OP_READ fires, i create a client object and
add it to the queue. Now OP_READ fires again for that same client send
(a thread did not process the read yet), and again a client object is
created and added to the queue for that same client.).

how do i prevent this? the one way is to make my Queue contain only
unique objects based on the Selection Key. but that is wasted
processing i think. i would prefer a way to say "i have queued this
event, please don't fire it again".

any thoughts.

 

[Owen Jacobson]

Looking for some "best practice" advice. The problem i am trying to solve
is the following:

I have a server that waits for OP_ACCEPT and when that happens, registers
OP_READ for that channel).
When an OP_READ event is fired, my handling routine creates a "client"
object (which contains the Selection Key as a member) and add this to a
synchronized "queue". I have a number of threads blocking on that queue
when it's empty...so that when something is added, one of the threads
picks it up and processes it (reads from the socket).

However, once an OP_READ is fired, it continues to be fired until a read
occurs from that socket channel i believe. This means that if a thread
happens to not read from the queue'd client i just added, that event is
fired again, and in this case, i could have more than one object added to
my queue for the same event (i.e. if a client socket is sending to my
server, when OP_READ fires, i create a client object and add it to the
queue. Now OP_READ fires again for that same client send (a thread did
not process the read yet), and again a client object is created and added
to the queue for that same client.).

how do i prevent this? the one way is to make my Queue contain only
unique objects based on the Selection Key. but that is wasted processing
i think. i would prefer a way to say "i have queued this event, please
don't fire it again".

any thoughts.

When iterating over the set of returned selection keys using an Iterator
i, remember to call i.remove (); after handling the event. Selectors
don't automatically "unset" event notification when you call select() on
them; if you don't remove a key from the selectedKeys set then it will
remain there until the key is cancelled completely.

This one caught me by surprise, too.

 

[farseer]

i do that already. it seems the problem is that the event will fire
until the buffer is read. that actually makes sense...but for
asynchronous processing of events (as described in my original post),
it's a pain...

 

[Owen Jacobson]

i do that already. it seems the problem is that the event will fire until
the buffer is read. that actually makes sense...but for asynchronous
processing of events (as described in my original post), it's a pain...

Sounds kind of "Doctor, it hurts when I do this!"-ish. The answer is not
to do it the way you're doing it: what about having the Selector handler
pull the data out of the channel into a buffer rather than having your
worker threads do the read? That way, when you call select() again,
you're guaranteed that read notifications arriving are *always* for
new data, rather than for data that may be new or old depending on the
behaviour of thread scheduling.

Think carefully about what your code is asking the system.

Program: "Wake me when any of these have data."
(Some time passes.)
System: "Wake up. The following channels have data."
Program: "Ok. I may or may not have read data from them yet. Wake me
when any of these have data."

Clearly, in cases when you haven't read the data, the data is still there
and the system will wake up your program right away (the channel has
data). However, if new data arrives, the exact same thing happens -- and
you can't tell the difference between the two cases.

An alternate approach, and I stress that this is one I would NOT use,
would be to disable the SelectionKey's interest in OP_READ when read is
fired and re-enable it after reading from the channel. While this looks
okay on the surface, it's one more synchronization headache to ensure that
interest is not re-enabled while the Selector is working with the key in
question.

 

[Esmond Pitt]

Sounds kind of "Doctor, it hurts when I do this!"-ish. The answer is not
to do it the way you're doing it: what about having the Selector handler
pull the data out of the channel into a buffer rather than having your
worker threads do the read? That way, when you call select() again,
you're guaranteed that read notifications arriving are *always* for
new data, rather than for data that may be new or old depending on the
behaviour of thread scheduling.

Think carefully about what your code is asking the system.

Program: "Wake me when any of these have data."
(Some time passes.)
System: "Wake up. The following channels have data."
Program: "Ok. I may or may not have read data from them yet. Wake me
when any of these have data."

Clearly, in cases when you haven't read the data, the data is still there
and the system will wake up your program right away (the channel has
data). However, if new data arrives, the exact same thing happens -- and
you can't tell the difference between the two cases.

An alternate approach, and I stress that this is one I would NOT use,
would be to disable the SelectionKey's interest in OP_READ when read is
fired and re-enable it after reading from the channel. While this looks
okay on the surface, it's one more synchronization headache to ensure that
interest is not re-enabled while the Selector is working with the key in
question.

I still don't understand how the OP is getting the same data onto his
queues twice. This implies that you can read the same data twice from a
channel, which is certainly untrue. OP_READ will keep firing as long as
there is data in the socket receive buffer; whether you read it all or a
piece or none is up to you. If duplicate data is being seen it can only
be a problem in the application logic.

 

[farseer]

Esmond,
no, in my orignal post, i pointed out that once an item is added to the
queue, a worker thread picks it up. HOwever, if there is a slight
delay in either picking up and processing that item, the event
(OP_READ) will fire again. As you stated, and as i suspect, it will
fire until it is read from the channel. that is the problem. there is
sometimes a race condition..in which the worker threads don't pick it
up right away and there fore it fires again and is added to the queue.

I think i might have a solution however...afer processing the key the
first time, i should be able to remove that key from the Ready Ops...i
haven't tried it, but aftger reading some more on this subject, i think
that might work.

 

[Owen Jacobson]

Esmond,
no, in my orignal post, i pointed out that once an item is added to the
queue, a worker thread picks it up. HOwever, if there is a slight delay
in either picking up and processing that item, the event (OP_READ) will
fire again. As you stated, and as i suspect, it will fire until it is
read from the channel. that is the problem. there is sometimes a race
condition..in which the worker threads don't pick it up right away and
there fore it fires again and is added to the queue.

I think i might have a solution however...afer processing the key the
first time, i should be able to remove that key from the Ready Ops...i
haven't tried it, but aftger reading some more on this subject, i think
that might work.

There is no way, barring bugs in NIO, that you can tell NIO not to raise
OP_READ notification on a channel that has data that is interested in
reading. Removing OP_READ from the ready set (should you find a way to do
that; the API doesn't provide a direct one) will simply cause OP_READ to
be re-added to the ready set at the next select.

There are three ways to get a selector not to raise OP_READ notification
for a channel that has data:

1. Cancel the selection key.
2. Disable interest in OP_READ.
3. Read the data, thus removing it from the channel.

The first option might work, but involves manipulating the selector's sets
of SelectionKeys, which is at *best* O(log n) complexity. The second
works, but reenabling OP_READ interest while another thread is in select()
is unsafe, so you'd have to build otherwise-useless synchronization
mechanisms. The third is the right way -- because the data is already
present, the selection thread doesn't need to block, and after the data
has been read further OP_READ notifications will always be for new data.

Rethink your architecture. Instead of having the workers start by reading
data from the channel, have the workers start by processing the data
already read by the selection thread.

Owen

 

[farseer]

the intention is to not have the event fire again UNTIL i finish
processing the buffer (which was just added to the queue as a result of
OP_READ).

the solution i eventually went with is exactly what you describe in #2,
disabling the interest after an OP_READ is fired and the object is
added to the queue. after the data has been processed (after i have
checked to see if data in the buffer constitute a complete packet based
on my "protocol"), i reable the READ interest. Regarding the point you
mention about re-enabling OP_READ in a different thread..i get around
this by actually doing this in the original thread. what i do is
"schedule" the re-enabling in the original thread it self. basically
there's a method in the original thread that adds a RUNNABLE object to
a list, then calls selector.wakeup. the first thing in the selector
loops is that it checks to see if there are any pending RUNNABLE
objects to execute. in this case, the Runnable object simply adds the
interest back to the channel. doing this, i can in actuallity ensure
ALL interaction with the selector and it's keys are done in the
original thread, the one with the selector loop.

I could have easily gone with #3, as i think you are correct, that will
work. but it would mean that i would have to maintain a separate
ByteBuffer for each client connection (channel), as it is not
guaranteed that the entire packet will be recieved with one read. I
also worry about synchronization with #3 and may still have to disable
OP_READ, otherwise it is possible for the buffer to be written to while
i am attempting to process the data in that buffer. what do you think,
is that true?

I really appreciate the feed back and advice. thus this is working out
well, but as i progress, should there be problems, i know i have
another alternatives to try and i know the person to ask...you've been
very helpful...thanks much again.

 

[Esmond Pitt]

There are three ways to get a selector not to raise OP_READ notification
for a channel that has data:

1. Cancel the selection key.
2. Disable interest in OP_READ.
3. Read the data, thus removing it from the channel.

The first option might work, but involves manipulating the selector's sets
of SelectionKeys, which is at *best* O(log n) complexity.

Owen

just curious, where do you get this O() from? I could understand O(N) if
N is the number of cancelled keys, or O(1) if N is the total number of
registered & cancelled keys for the selector, but O(log N)?

EJP

 

[Owen Jacobson]

Owen

just curious, where do you get this O() from? I could understand O(N) if N
is the number of cancelled keys, or O(1) if N is the total number of
registered & cancelled keys for the selector, but O(log N)?

N in this case is the number of keys registered.

From java.util.TreeSet<E>. "This implementation provides guaranteed
log(n) time cost for the basic operations (add, remove and contains)."
Therefore, if the Sets that Selector uses are TreeSet<E>s, cancelling a
key involves adding the key to the cancelled key set and, on the next
select, removing it from the registered and possibly from the ready key
set. (And, of course, O(2 * log n) is O(log n).) Adding a channel
involves adding it to at least the registered set.

That's not strictly true, though, because I forgot that HashSet<E> is O(1)
in the general case with exceptions for when it needs to expand the hash
table.

Dunno what it actually uses, and don't particularly care.

 

[Esmond Pitt]

N in this case is the number of keys registered.

From java.util.TreeSet<E>. "This implementation provides guaranteed
log(n) time cost for the basic operations (add, remove and contains)."
Therefore, if the Sets that Selector uses are TreeSet<E>s, cancelling a
key involves adding the key to the cancelled key set and, on the next
select, removing it from the registered and possibly from the ready key
set. (And, of course, O(2 * log n) is O(log n).) Adding a channel
involves adding it to at least the registered set.

That's not strictly true, though, because I forgot that HashSet<E> is O(1)
in the general case with exceptions for when it needs to expand the hash
table.

Dunno what it actually uses, and don't particularly care.

Interesting trail of futile speculation, but for those who care it does
use HashSet and it is therefore O(1). But you're right, why care? just
guess away!