[tarantool-patches] Re: [PATCH] vinyl: don't yield in on_commit and on_rollback triggers
Vladimir Davydov
vdavydov.dev at gmail.com
Thu Jun 20 14:18:00 MSK 2019
On Wed, Jun 19, 2019 at 09:23:29PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev at gmail.com> [19/06/16 16:11]:
>
> Basically the patch is LGTM but I would not use a mempool
> vy_log_tx but more strongly a a region for vy_log records - these
> are rare events (less than a handful a second per instance) and
> using a malloc for such is entirely justified.
>
> On the other hand I don't see any harm from using these (memory is
> not pinned for along period of time, for example), so please feel
> free to not bother with the comment.
>
> > +vy_log_flusher_func(va_list va)
>
> The convention is that we simply write _f for fiber functions at the
> end.
>
> I also did not find the place where you cancel or kill the fiber -
> you assume it runs forever - please document it, since otherwise
> the infinite fiber loop looks confusing.
Pushed to master with the following changes:
diff --git a/src/box/vy_log.c b/src/box/vy_log.c
index ae1d6234..098a0141 100644
--- a/src/box/vy_log.c
+++ b/src/box/vy_log.c
@@ -159,7 +159,13 @@ struct vy_log {
* log rotation.
*/
struct latch latch;
- /** Background fiber flushing pending transactions. */
+ /**
+ * Background fiber flushing pending transactions.
+ * Lives throughout the vinyl engine lifetime. Note,
+ * we don't stop it in destructor, because the event
+ * loop is dead at that time so we can't properly
+ * join it.
+ */
struct fiber *flusher;
/** Condition variable used for signalling the flusher. */
struct fiber_cond flusher_cond;
@@ -196,7 +202,7 @@ struct vy_log {
static struct vy_log vy_log;
static int
-vy_log_flusher_func(va_list va);
+vy_log_flusher_f(va_list va);
static struct vy_recovery *
vy_recovery_new_locked(int64_t signature, int flags);
@@ -761,7 +767,7 @@ vy_log_init(const char *dir)
wal_init_vy_log();
fiber_cond_create(&vy_log.flusher_cond);
vy_log.flusher = fiber_new("vinyl.vylog_flusher",
- vy_log_flusher_func);
+ vy_log_flusher_f);
if (vy_log.flusher == NULL)
panic("failed to allocate vylog flusher fiber");
fiber_wakeup(vy_log.flusher);
@@ -887,7 +893,7 @@ vy_log_flush(void)
}
static int
-vy_log_flusher_func(va_list va)
+vy_log_flusher_f(va_list va)
{
(void)va;
while (!fiber_is_cancelled()) {
More information about the Tarantool-patches
mailing list