[ovs-dev] ovsdb: Fix database compaction check

We want to compact database file if it has been over 24 hours since we
last compacted it and there's more than 100 commits regardless of the
size of the database. This patch fixes the previous comparisson which
checked if 24 hours was elapsed since the next scheduled compaction.

Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
---
 ovsdb/file.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Thanks Daniel

Acked-by: Mark Michelson <mmichels@redhat.com>

On 03/10/2018 07:48 AM, Daniel Alvarez wrote:
> We want to compact database file if it has been over 24 hours since we
> last compacted it and there's more than 100 commits regardless of the
> size of the database. This patch fixes the previous comparisson which
> checked if 24 hours was elapsed since the next scheduled compaction.
> 
> Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
> ---
>   ovsdb/file.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/ovsdb/file.c b/ovsdb/file.c
> index 02e0e8b76..333cd00a6 100644
> --- a/ovsdb/file.c
> +++ b/ovsdb/file.c
> @@ -611,12 +611,12 @@ ovsdb_file_commit(struct ovsdb_replica *replica,
>        * tried), and if there are at least 100 transactions in the database, and
>        * if the database is at least 10 MB, and the database is at least 2x the
>        * size of the previous snapshot, then compact the database. However, if
> -     * it has been over COMPACT_MAX_MSEC ms, the database size is not taken
> -     * into account. */
> -    next_compact_elapsed = time_msec() - file->next_compact;
> -    if (next_compact_elapsed >= 0
> +     * it has been over COMPACT_MAX_MSEC ms since the last compaction, the
> +     * database size is not taken into account. */
> +    current_time = time_msec();
> +    if (current_time >= file->next_compact
>           && file->n_transactions >= 100
> -        && (next_compact_elapsed >= COMPACT_MAX_MSEC
> +        && (curent_time - file->last_compact >= COMPACT_MAX_MSEC
>               || ovsdb_log_grew_lots(file->log))) {
>           error = ovsdb_file_compact(file);
>           if (error) {
>

On Sat, Mar 10, 2018 at 02:48:00PM +0100, Daniel Alvarez wrote:
> We want to compact database file if it has been over 24 hours since we
> last compacted it and there's more than 100 commits regardless of the
> size of the database. This patch fixes the previous comparisson which
> checked if 24 hours was elapsed since the next scheduled compaction.
> 
> Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>

I think I'm missing a prerequisite or maybe this has not been
compile-tested:

    ../ovsdb/file.c:616:5: error: use of undeclared identifier 'current_time'
    ../ovsdb/file.c:617:9: error: use of undeclared identifier 'current_time'
    ../ovsdb/file.c:619:13: error: use of undeclared identifier 'curent_time'

?

On Wed, Mar 14, 2018 at 03:13:33PM -0700, Ben Pfaff wrote:
> On Sat, Mar 10, 2018 at 02:48:00PM +0100, Daniel Alvarez wrote:
> > We want to compact database file if it has been over 24 hours since we
> > last compacted it and there's more than 100 commits regardless of the
> > size of the database. This patch fixes the previous comparisson which
> > checked if 24 hours was elapsed since the next scheduled compaction.
> > 
> > Signed-off-by: Daniel Alvarez <dalvarez@redhat.com>
> 
> I think I'm missing a prerequisite or maybe this has not been
> compile-tested:
> 
>     ../ovsdb/file.c:616:5: error: use of undeclared identifier 'current_time'
>     ../ovsdb/file.c:617:9: error: use of undeclared identifier 'current_time'
>     ../ovsdb/file.c:619:13: error: use of undeclared identifier 'curent_time'
> 
> ?

Oops, sorry.  I see v2 now.