[Tarantool-patches] [PATCH v3] wal: introduce limits on simultaneous writes

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

I must admit, it looks kind of ugly :D. The class we have now only remotely
looks like a semaphore.

Number of reasons, some of which you already listed in the header file:

- It is advisory. Can be bypassed easily if you forget to check wouldblock.
  But not a big issue really. An optional thing like 'try_take' is needed
  for box.commit({is_async = true}) anyway, not to block the fiber;

- You can take more amount of the resource than there is. Bearable as well,
  but still;

- sem_release() does not wakeup anybody. Quite counter-intuitive;

- The wouldblock check not only checks the resource being available, but also
  if there are any waiters. It wouldn't matter for a real semaphore, because
  it has nothing to do with ordering the waiters in FIFO. It is a detail of
  the journal which slipped into the general class.
  But maybe that is the only way to make it fair? Otherwise some fibers
  could be blocked forever due to starvation.

The last thing I am not sure is even an issue. Might be a feature.

The others probably can be fixed if we would rework journal_queue API. For
instance, not have journal_queue_wait() separated from journal_queue_on_append().
Then sem_take() could become blocking and obligatory.

You simply inline everything into journal_write() and journal_write_try_async(),
and you will see that you can always call take() and block inside of it.

But I don't know if it is worth doing TBH. It is used in a single place so far.
This is hard to define fiber_sem API which would be suitable for future usages.
I would vote for not doing it now and see if we would need the semaphore in the
future.

Although the idea about removing journal_queue_wait() might be worth trying.
It is used either right before journal_queue_on_append(), or in
journal_queue_flush() which is also right before journal_queue_on_append().
Up to you. Anyway we need to return to this code for box.commit({is_async})
feature, which means the hard polishing might be not so useful.