From: Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Cyrill Gorcunov <gorcunov@gmail.com>, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tml <tarantool-patches@dev.tarantool.org> Subject: Re: [Tarantool-patches] [PATCH 1/3] wal: sanitize wal_mode Date: Sun, 25 Apr 2021 11:08:25 +0300 [thread overview] Message-ID: <48231b42-debd-ac18-2a97-bbfa9b4aec5b@tarantool.org> (raw) In-Reply-To: <YIM5TbZWe4GChojQ@grain> 24.04.2021 00:17, Cyrill Gorcunov via Tarantool-patches пишет: > On Fri, Apr 23, 2021 at 10:57:11PM +0200, Vladislav Shpilevoy wrote: >> Hi! Thanks for the patch! >> >>> diff --git a/src/box/wal.h b/src/box/wal.h >>> index a8a16ba2e..470aa1cc8 100644 >>> --- a/src/box/wal.h >>> +++ b/src/box/wal.h >>> @@ -41,7 +41,29 @@ struct fiber; >>> struct wal_writer; >>> struct tt_uuid; >>> >>> -enum wal_mode { WAL_NONE = 0, WAL_WRITE, WAL_FSYNC, WAL_MODE_MAX }; >>> +enum wal_mode { >>> + /** >>> + * Do not write data at all. >>> + */ >>> + WAL_NONE, >>> + >>> + /** >>> + * Write without waiting the data to be >>> + * flushed to the storage device. >>> + */ >>> + WAL_WRITE, >>> + >>> + /** >>> + * Write data and wait the record to be >>> + * flushed to the storage device. >>> + */ >>> + WAL_FSYNC, >>> + >>> + WAL_MODE_MAX >>> +}; >>> + >>> +/** String constants for the supported modes. */ >>> +extern const char *wal_mode_STRS[WAL_MODE_MAX]; >> 1. Why do you need to declare in the header the array size? >> The old version with [] was just fine. And less places to change >> if becomes necessary. > If the size is known better to point it explicitly. Thus when you > read this code you will notise the array size (yes, it doesn't > matter from compiler pov if we declare it as a double pointer > but still). But fine I'll leave this declaration untouched. > >> 2. Please, keep the global variable where it was. We usually group >> global variable declarations. It was grouped with wal_dir_lock >> below. > --- > From: Cyrill Gorcunov <gorcunov@gmail.com> > Date: Fri, 16 Apr 2021 15:54:52 +0300 > Subject: [PATCH] wal: sanitize wal_mode > > - there is no need to set WAN_NONE to 0 explicitly > - add comments about modes > - there is no need to carry NULL in wal_mode_STRS > - use designated assignment in wal_mode_STRS > > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com> > --- > src/box/wal.c | 6 +++++- > src/box/wal.h | 21 ++++++++++++++++++++- > 2 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/src/box/wal.c b/src/box/wal.c > index 5b6200b81..6468df884 100644 > --- a/src/box/wal.c > +++ b/src/box/wal.c > @@ -55,7 +55,11 @@ enum { > WAL_FALLOCATE_LEN = 1024 * 1024, > }; > > -const char *wal_mode_STRS[] = { "none", "write", "fsync", NULL }; > +const char *wal_mode_STRS[WAL_MODE_MAX] = { > + [WAL_NONE] = "none", > + [WAL_WRITE] = "write", > + [WAL_FSYNC] = "fsync", > +}; > > int wal_dir_lock = -1; > > diff --git a/src/box/wal.h b/src/box/wal.h > index a8a16ba2e..36fcd39bd 100644 > --- a/src/box/wal.h > +++ b/src/box/wal.h > @@ -41,7 +41,26 @@ struct fiber; > struct wal_writer; > struct tt_uuid; > > -enum wal_mode { WAL_NONE = 0, WAL_WRITE, WAL_FSYNC, WAL_MODE_MAX }; > +enum wal_mode { > + /** > + * Do not write data at all. > + */ > + WAL_NONE, > + > + /** > + * Write without waiting the data to be > + * flushed to the storage device. > + */ > + WAL_WRITE, > + > + /** > + * Write data and wait the record to be > + * flushed to the storage device. > + */ > + WAL_FSYNC, > + > + WAL_MODE_MAX > +}; > Thanks for the patch! I'd leave an explicit WAL_NONE = 0. I didn't know that enums start at zero, TBH. This would be more readable IMO. Up to you. > enum { > /** -- Serge Petrenko
next prev parent reply other threads:[~2021-04-25 8:08 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-19 8:37 [Tarantool-patches] [PATCH 0/3] wal,txn_limbo: a few cleanups Cyrill Gorcunov via Tarantool-patches 2021-04-19 8:37 ` [Tarantool-patches] [PATCH 1/3] wal: sanitize wal_mode Cyrill Gorcunov via Tarantool-patches 2021-04-23 20:57 ` Vladislav Shpilevoy via Tarantool-patches 2021-04-23 21:17 ` Cyrill Gorcunov via Tarantool-patches 2021-04-25 8:08 ` Serge Petrenko via Tarantool-patches [this message] 2021-04-25 13:26 ` Cyrill Gorcunov via Tarantool-patches 2021-04-26 9:40 ` Serge Petrenko via Tarantool-patches 2021-04-26 9:58 ` Cyrill Gorcunov via Tarantool-patches 2021-04-19 8:37 ` [Tarantool-patches] [PATCH 2/3] txn_limbo: simplify owner migration condition Cyrill Gorcunov via Tarantool-patches 2021-04-25 8:09 ` Serge Petrenko via Tarantool-patches 2021-04-19 8:37 ` [Tarantool-patches] [PATCH 3/3] txn_limbo: simplify txn_limbo_ack Cyrill Gorcunov via Tarantool-patches 2021-04-25 8:11 ` Serge Petrenko via Tarantool-patches 2021-04-22 11:34 ` [Tarantool-patches] [PATCH 4/4] txn_limbo: simplify txn_limbo_on_parameters_change Cyrill Gorcunov via Tarantool-patches 2021-04-25 8:13 ` Serge Petrenko via Tarantool-patches 2021-04-25 13:18 ` [Tarantool-patches] [PATCH 0/3] wal,txn_limbo: a few cleanups Vladislav Shpilevoy via Tarantool-patches 2021-04-26 15:40 ` Kirill Yukhin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=48231b42-debd-ac18-2a97-bbfa9b4aec5b@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=gorcunov@gmail.com \ --cc=sergepetrenko@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 1/3] wal: sanitize wal_mode' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox