On Fri, Mar 09, 2018 at 08:05:22AM -0600, Mark Michelson wrote: > Hi Daniel, > > Mostly this looks correct. I had one small finding and have noted it in-line > down below. > > On 03/08/2018 04:20 PM, Daniel Alvarez wrote: > >Before this patch, the databases were automatically compacted when a > >transaction is logged when: > > > >* It's been > 10 minutes after last compaction AND > >* At least 100 commits have occurred AND > >* Database has grown at least 4x since last compaction (and it's > 10M) > > > >This patch changes the conditions as follows: > > > >* It's been > 10 minutes after last compaction AND > >* At least 100 commits have occurred AND either > > - It's been > 24 hours after the last compaction OR > > - Database has grown at least 2x since last compaction (and it's > 10M) > > > >Reported-by: Daniel Alvarez <[email protected]> > >Reported-at: > >https://mail.openvswitch.org/pipermail/ovs-discuss/2018-March/046309.html > >Signed-off-by: Daniel Alvarez <[email protected]> > >--- > > ovsdb/file.c | 17 +++++++++++++---- > > ovsdb/log.c | 2 +- > > ovsdb/ovsdb-server.1.in | 6 ++++-- > > 3 files changed, 18 insertions(+), 7 deletions(-) > > > >diff --git a/ovsdb/file.c b/ovsdb/file.c > >index 4b7ad52ab..02e0e8b76 100644 > >--- a/ovsdb/file.c > >+++ b/ovsdb/file.c > >@@ -46,6 +46,10 @@ VLOG_DEFINE_THIS_MODULE(ovsdb_file); > > * compacting fails. */ > > #define COMPACT_RETRY_MSEC (60 * 1000) /* 1 minute. */ > >+/* Maximum number of milliseconds before compacting the database regardless > >+ * of its size. */ > >+#define COMPACT_MAX_MSEC (24 * 60 * 60 * 1000) /* 24 hours. */ > >+ > > /* A transaction being converted to JSON for writing to a file. */ > > struct ovsdb_file_txn { > > struct json *json; /* JSON for the whole transaction. */ > >@@ -586,6 +590,7 @@ ovsdb_file_commit(struct ovsdb_replica *replica, > > struct ovsdb_file *file = ovsdb_file_cast(replica); > > struct ovsdb_file_txn ftxn; > > struct ovsdb_error *error; > >+ long long int next_compact_elapsed; > > ovsdb_file_txn_init(&ftxn); > > ovsdb_txn_for_each_change(txn, ovsdb_file_change_cb, &ftxn); > >@@ -604,11 +609,15 @@ ovsdb_file_commit(struct ovsdb_replica *replica, > > /* If it has been at least COMPACT_MIN_MSEC ms since the last time we > > * compacted (or at least COMPACT_RETRY_MSEC ms since the last time we > > * 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 4x > >the > >- * size of the previous snapshot, then compact the database. */ > >- if (time_msec() >= file->next_compact > >+ * 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 > > && file->n_transactions >= 100 > >- && ovsdb_log_grew_lots(file->log)) { > >+ && (next_compact_elapsed >= COMPACT_MAX_MSEC > > I think this line is not right. If the idea is to compact if it has been > more than 24 hours since the last time we did, then we should be doing > > time_msec() - file->last_compact >= COMPACT_MAX_MSEC > > instead of > > time_msec() - file->next_compact >= COMPACT_MAX_MSEC
Oops, I missed this before I committed. Daniel, would you mind sending a fix? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
