What operations are atomic in a coroutine?

kannan · 2017-02-22T08:36:53+00:00

Hello everyone,I'm trying to catch various types of errors on upstream and to show users the real cause of the problem (more detailed then showing o...

What operations are atomic in a coroutine?

kannan

I have the following setup:

1. In each request's `log_by_lua`, I record stats into a per-worker table. This typically involves operations like:

    worker_stats['num_requests'] += 1
    worker_stats['total_request_time'] += ...

2. I'm using a per-worker background task to periodically send those stats to another server.

    local last_batch = worker_stats
    worker_stats = {}
    send_stats_to_server(last_batch)
    // schedule this to run again in 100ms with nginx.timer.at

To make sure the data is correct, I need certain operations to be atomic. In the `log_by_lua` I need the increment operations to be individually atomic, but it would be nice if the whole block of increment operations were atomic. In the background task, I need the first two lines to be atomic.

Are things already guaranteed to be atomic (within the worker) as long as I don't explicitly yield or perform I/O? If not, is there something I can do to make sure my stats are tracked precisely?

Thanks!

rpaprocki

Hi,

On Tue, Feb 21, 2017 at 4:36 PM, Kannan Goundan <ka...@cakoose.com> wrote:

I have the following setup:

1. In each request's `log_by_lua`, I record stats into a per-worker table. This typically involves operations like:

    worker_stats['num_requests'] += 1
    worker_stats['total_request_time'] += ...

2. I'm using a per-worker background task to periodically send those stats to another server.

    local last_batch = worker_stats
    worker_stats = {}
    send_stats_to_server(last_batch)
    // schedule this to run again in 100ms with nginx.timer.at

To make sure the data is correct, I need certain operations to be atomic. In the `log_by_lua` I need the increment operations to be individually atomic, but it would be nice if the whole block of increment operations were atomic. In the background task, I need the first two lines to be atomic.

Are things already guaranteed to be atomic (within the worker) as long as I don't explicitly yield or perform I/O? If not, is there something I can do to make sure my stats are tracked precisely?

A good reference implementation for something like this is Cloudflare's socket logger module: https://github.com/cloudflare/lua-resty-logger-socket/. They use a per-worker log (just a binary flag) to lock socket access, and two distinct buffers to hold and ship data.

In theory, operations should be atomic because of the cooperative threading model in play. Both the increments in the first example, and everything before a yielding or socket operation in the second example (e.g. something inside send_stats_to_server()) should be executed without interruption on the worker. I implemented something similar for an Influx client writer (https://github.com/p0pr0ck5/lua-resty-influx/blob/master/lib/resty/influx/buffer.lua#L62) to try to do this as cheaply as possible, but got reports that high concurrency caused socket operations issues, so that approach might need some fine tuning. For now probably best to stick to the socket logger lib mentioned above :)

Tangentially, have a look at the "table.clear()" function available in LuaJIT 2.1, which is likely a better solution that just "worker_stats = {}".

kannan

Thanks for the pointers, Robert!

In theory, operations should be atomic because of the cooperative threading model in play ...

... but got reports that high concurrency caused socket operations issues, so that approach might need some fine tuning.

What kind of issues did you see? Did you see anything that indicated coroutines were interrupted (without yielding or performing I/O)?

On Tue, Feb 21, 2017 at 5:10 PM, Robert Paprocki <rpap...@fearnothingproductions.net> wrote:

Hi,

On Tue, Feb 21, 2017 at 4:36 PM, Kannan Goundan <ka...@cakoose.com> wrote:
I have the following setup:

1. In each request's `log_by_lua`, I record stats into a per-worker table. This typically involves operations like:

    worker_stats['num_requests'] += 1
    worker_stats['total_request_time'] += ...

2. I'm using a per-worker background task to periodically send those stats to another server.

    local last_batch = worker_stats
    worker_stats = {}
    send_stats_to_server(last_batch)
    // schedule this to run again in 100ms with nginx.timer.at

To make sure the data is correct, I need certain operations to be atomic. In the `log_by_lua` I need the increment operations to be individually atomic, but it would be nice if the whole block of increment operations were atomic. In the background task, I need the first two lines to be atomic.

Are things already guaranteed to be atomic (within the worker) as long as I don't explicitly yield or perform I/O? If not, is there something I can do to make sure my stats are tracked precisely?

A good reference implementation for something like this is Cloudflare's socket logger module: https://github.com/cloudflare/lua-resty-logger-socket/. They use a per-worker log (just a binary flag) to lock socket access, and two distinct buffers to hold and ship data.

In theory, operations should be atomic because of the cooperative threading model in play. Both the increments in the first example, and everything before a yielding or socket operation in the second example (e.g. something inside send_stats_to_server()) should be executed without interruption on the worker. I implemented something similar for an Influx client writer (https://github.com/p0pr0ck5/lua-resty-influx/blob/master/lib/resty/influx/buffer.lua#L62) to try to do this as cheaply as possible, but got reports that high concurrency caused socket operations issues, so that approach might need some fine tuning. For now probably best to stick to the socket logger lib mentioned above :)

Tangentially, have a look at the "table.clear()" function available in LuaJIT 2.1, which is likely a better solution that just "worker_stats = {}".

.

rpaprocki

Hi,

On Tue, Feb 21, 2017 at 6:42 PM, Kannan Goundan <ka...@cakoose.com> wrote:

Thanks for the pointers, Robert!

In theory, operations should be atomic because of the cooperative threading model in play ...

... but got reports that high concurrency caused socket operations issues, so that approach might need some fine tuning.

What kind of issues did you see? Did you see anything that indicated coroutines were interrupted (without yielding or performing I/O)?

The issue reported can be found at https://github.com/p0pr0ck5/lua-resty-influx/issues/2

It's sparse on details, so attributing a cause is conjecture, without contacting the issue author. I haven't yet tried to replicate this, nor have I deploy this lib in a high-concurrency environment, so I haven't dug deeply into it, but the data provided doesn't indicate thread interruption, only errors in performing socket operations, which is outside the boundary of this discussion. And it's possible that the reported errors are unrelated to the design of the lib entirely. But it seems out of scope for the discussion here (thread yielding).

agentzh

Hello!

On Tue, Feb 21, 2017 at 6:42 PM, Kannan Goundan wrote:
\>> In theory, operations should be atomic because of the cooperative
>> threading model in play ...
\>> ... but got reports that high concurrency caused socket operations issues,
>> so that approach might need some fine tuning.
>
>
> What kind of issues did you see?  Did you see anything that indicated
> coroutines were interrupted (without yielding or performing I/O)?
>

No, there cannot be any race conditions in design. The error log
messages in that github issue referenced are completely harmless and
modern versions of ngx_lua and OpenResty already suppressed those
alert level error messages so you should not see them any more.

See my comment in that github issue thread:
https://github.com/p0pr0ck5/lua-resty-influx/issues/2#issuecomment-316482989

Nothing but UNIX signals and OS process scheduler can forcibly
interrupt a running coroutine since nginx workers are all
single-threaded (if not counting the nginx OS thread pool, but even
that is a separate mechanism and is irrelevant here).

Best regards,
Yichun