Re: Thread Safety of Punctuator Functions and Processor Contexts

[Matthias J. Sax]

Good point about the docs... I guess it must support concurrency do to 
IQ, which might iterator over the store while `process()` modifies it. I 
was only reasoning about `process()` and punctuation and forgot IQ.


So it seems we indeed have this contract -- what is good new for you.

However, I don't think that there is any guarantee that you might "see" 
concurrent modification (IIRC, RocksDB uses snapshot isolation for 
iterators). But maybe that's good enough for you?



-Matthias


On 11/7/22 11:13 AM, Joshua Suskalo wrote:

"Matthias J. Sax"  writes:


In general, it's not safe to keep the iterator open, because when process() is
executed in-between two punctuator calls, it might modify the store and
invalidate the iterator. There is no guarantee that the returned iterator
supports concurrency.


This makes sense but unfortunately adds significant challenge to making these
operations happen concurrently, which is effectively a hard requirement for my
usecase due to the scale of the state stores' contained data.


Hence, even if it happens that the currently used iterator is concurrent, there
is no API contract about it.


This surprises me though, because it seems to contratict what's stated in the
ReadOnlyKeyValueStore documentation[1].


The returned iterator must be safe from ConcurrentModificationExceptions and
must not return null values. Order is not guaranteed as bytes lexicographical
ordering might not represent key order.


Maybe I'm reading this wrong, but it seems to imply that the returned iterator
must be safe in the face of concurrent modifications, which is required if you
are permitted to make a read-modify-write cycle while using the iterator, which
the existing version of my application has been doing correctly so far as I can
tell.

Am I vastly misunderstanding the intended usecase for punctuators and need to
determine a different mechanism for performing periodic operations on a data
store?

[1]: 

Re: Thread Safety of Punctuator Functions and Processor Contexts

[Joshua Suskalo]

"Matthias J. Sax"  writes:

> In general, it's not safe to keep the iterator open, because when process() is
> executed in-between two punctuator calls, it might modify the store and
> invalidate the iterator. There is no guarantee that the returned iterator
> supports concurrency.

This makes sense but unfortunately adds significant challenge to making these
operations happen concurrently, which is effectively a hard requirement for my
usecase due to the scale of the state stores' contained data.

> Hence, even if it happens that the currently used iterator is concurrent, 
> there
> is no API contract about it.

This surprises me though, because it seems to contratict what's stated in the
ReadOnlyKeyValueStore documentation[1].

> The returned iterator must be safe from ConcurrentModificationExceptions and
> must not return null values. Order is not guaranteed as bytes lexicographical
> ordering might not represent key order.

Maybe I'm reading this wrong, but it seems to imply that the returned iterator
must be safe in the face of concurrent modifications, which is required if you
are permitted to make a read-modify-write cycle while using the iterator, which
the existing version of my application has been doing correctly so far as I can
tell.

Am I vastly misunderstanding the intended usecase for punctuators and need to
determine a different mechanism for performing periodic operations on a data
store?

[1]: 

Re: Thread Safety of Punctuator Functions and Processor Contexts

[Matthias J. Sax]

In general, it's not safe to keep the iterator open, because when 
process() is executed in-between two punctuator calls, it might modify 
the store and invalidate the iterator. There is no guarantee that the 
returned iterator supports concurrency.


Hence, even if it happens that the currently used iterator is 
concurrent, there is no API contract about it.


-Matthias

On 11/7/22 7:41 AM, Joshua Suskalo wrote:

Hello Matthias, thanks for the response!


"Matthias J. Sax"  writes:


Spanning your own thread and calling context.forward() is _not_ safe, and there
is currently no way for you to make is safe. The runtime code makes certain
assumptions about being single threaded which would break if you call
context.forward() from a different thread. (The runtime _always_ assume that
context.forward() is called inside process() or inside the punctuation
callback.)


This is how I expected, so I'm not too concerned here.


The only way forward I can see, would be trying to make the punctuation call
shorter, eg, not scanning the full store but only a small part of it, such that
the thread can go back to execute process() quicker (it's of course an
additional challenge to keep track where you stopped the scan and to resume
it...), and to make the punctuation interval shorter.


This is where I have another question about safety. Is it safe to use a
KVStoreIterator that was retrieved during a punctuator after that punctuator
call has exited, perhaps using it to store offset information across multiple
runs? This seems to me to be the most obvious way to handle this.

A related question if that one has an affirmative answer would be asking if the
KVStoreIterator can be safely sent to another thread for use (akin to Rust's
Send trait, in that I do not wish to have concurrent access, only to use it from
another thread), as if that were possible I could use the j.u.c.ConcurrentQueue
to allow a thread created on the punctuator to compute the messages that need to
be sent and enqueue them, and then in further punctuator runs I could then send
messages which have been computed.


Hope this helps.


What you've said already is quite helpful and I will begin pursuing methods
based on what you've said, but I also hope that someone can answer my further
questions since they will be helpful to my implementation as well.

Thanks so much,
Joshua

Re: Thread Safety of Punctuator Functions and Processor Contexts

[Joshua Suskalo]

Hello Matthias, thanks for the response!


"Matthias J. Sax"  writes:

> Spanning your own thread and calling context.forward() is _not_ safe, and 
> there
> is currently no way for you to make is safe. The runtime code makes certain
> assumptions about being single threaded which would break if you call
> context.forward() from a different thread. (The runtime _always_ assume that
> context.forward() is called inside process() or inside the punctuation
> callback.)

This is how I expected, so I'm not too concerned here.

> The only way forward I can see, would be trying to make the punctuation call
> shorter, eg, not scanning the full store but only a small part of it, such 
> that
> the thread can go back to execute process() quicker (it's of course an
> additional challenge to keep track where you stopped the scan and to resume
> it...), and to make the punctuation interval shorter.

This is where I have another question about safety. Is it safe to use a
KVStoreIterator that was retrieved during a punctuator after that punctuator
call has exited, perhaps using it to store offset information across multiple
runs? This seems to me to be the most obvious way to handle this.

A related question if that one has an affirmative answer would be asking if the
KVStoreIterator can be safely sent to another thread for use (akin to Rust's
Send trait, in that I do not wish to have concurrent access, only to use it from
another thread), as if that were possible I could use the j.u.c.ConcurrentQueue
to allow a thread created on the punctuator to compute the messages that need to
be sent and enqueue them, and then in further punctuator runs I could then send
messages which have been computed.

> Hope this helps.

What you've said already is quite helpful and I will begin pursuing methods
based on what you've said, but I also hope that someone can answer my further
questions since they will be helpful to my implementation as well.

Thanks so much,
Joshua