| Message ID | 20240110192939.15220-7-frode.nordahl@canonical.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | Introduce cooperative multitasking to improve OVSDB RAFT cluster operation. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | fail | test: fail |
On 1/10/24 20:29, Frode Nordahl wrote: > Initialize the cooperative multitasking module for the > ovsdb-server. > > The server side schema conversion process used for storage > engines such as RAFT is time consuming, yield during > processing. > > After the schema conversion is done, the processing of JSON-RPC > sessions and OVSDB monitors for reconnecting clients can > overrun the configured election timer. > > The JSON serialization of the database contents has been > identified as one of the primary offenders. Make use of > yielding versions of JSON object create and destroy functions > to mitigate. > > This series has been tested by checking success of schema > conversion, ensuring no involuntary leader change occurs with > election timer configurations as low as 750 msec, on a 75MB > database with ~ 100 connected clients as produced by the > ovn-heater ocp-120-density-light test scenario. Nice! > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > --- > NEWS | 8 ++++++++ > ovsdb/file.c | 2 ++ > ovsdb/jsonrpc-server.c | 2 ++ > ovsdb/monitor.c | 12 ++++++++---- > ovsdb/ovsdb-server.c | 5 +++++ > ovsdb/trigger.c | 3 +++ > 6 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/NEWS b/NEWS > index 270ed6673..643fddc98 100644 > --- a/NEWS > +++ b/NEWS > @@ -6,6 +6,9 @@ Post-v3.2.0 > from older version is supported but it may trigger more leader elections > during the process, and error logs complaining unrecognized fields may > be observed on old nodes. > + * Make use of cooperative multitasking to ensure stable maintenance of > + RAFT cluster during long running processing such as online schema > + conversion. > - OpenFlow: > * NXT_CT_FLUSH extension is updated to support flushing connections > based on mark and labels. 'ct-flush' command of ovs-ofctl updated > @@ -36,6 +39,11 @@ Post-v3.2.0 > The existing behaviour is maintained and a non key:value pair value > will be applied to all other PMD thread cores.'pmd-sleep-show' is > updated to show the maximum sleep for each PMD thread core. > + - lib: > + * Introduce cooperative multitasking module which allow us to interleave > + important processing with long running tasks while avoiding the > + additional resource consumption of threads and complexity of > + asynchronous state machines. NEWS are mostly for user-visible changes. They don't need to know about a new internal library. Entry in the OVSDB section is enough. > > > v3.2.0 - 17 Aug 2023 > diff --git a/ovsdb/file.c b/ovsdb/file.c > index 8bd1d4af3..fb4cbfb77 100644 > --- a/ovsdb/file.c > +++ b/ovsdb/file.c > @@ -23,6 +23,7 @@ > > #include "bitmap.h" > #include "column.h" > +#include "cooperative-multitasking.h" > #include "log.h" > #include "openvswitch/json.h" > #include "lockfile.h" > @@ -308,6 +309,7 @@ ovsdb_convert_table(struct ovsdb_txn *txn, > } > > HMAP_FOR_EACH (src_row, hmap_node, &src_table->rows) { > + cooperative_multitasking_yield(); Please, move this after the empty line below variable declaration. > struct ovsdb_row *dst_row = ovsdb_row_create(dst_table); > *ovsdb_row_get_uuid_rw(dst_row) = *ovsdb_row_get_uuid(src_row); > > diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c > index 45f7c8038..f08b5a946 100644 > --- a/ovsdb/jsonrpc-server.c > +++ b/ovsdb/jsonrpc-server.c > @@ -21,6 +21,7 @@ > > #include "bitmap.h" > #include "column.h" > +#include "cooperative-multitasking.h" > #include "openvswitch/dynamic-string.h" > #include "monitor.h" > #include "openvswitch/json.h" > @@ -600,6 +601,7 @@ ovsdb_jsonrpc_session_run_all(struct ovsdb_jsonrpc_remote *remote) > struct ovsdb_jsonrpc_session *s; > > LIST_FOR_EACH_SAFE (s, node, &remote->sessions) { > + cooperative_multitasking_yield(); An empty line here would be nice. > int error = ovsdb_jsonrpc_session_run(s); > if (error) { > ovsdb_jsonrpc_session_close(s); > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c > index d1e466faa..93cc66b76 100644 > --- a/ovsdb/monitor.c > +++ b/ovsdb/monitor.c > @@ -20,6 +20,7 @@ > > #include "bitmap.h" > #include "column.h" > +#include "cooperative-multitasking.h" > #include "openvswitch/dynamic-string.h" > #include "openvswitch/json.h" > #include "jsonrpc.h" > @@ -262,7 +263,7 @@ ovsdb_monitor_json_cache_flush(struct ovsdb_monitor *dbmon) > struct ovsdb_monitor_json_cache_node *node; > > HMAP_FOR_EACH_POP(node, hmap_node, &dbmon->json_cache) { > - json_destroy(node->json); > + json_destroy_with_yield(node->json); > free(node); > } > } > @@ -278,7 +279,7 @@ ovsdb_monitor_json_cache_destroy(struct ovsdb_monitor *dbmon, > = ovsdb_monitor_json_cache_search(dbmon, v, change_set); > if (node) { > hmap_remove(&dbmon->json_cache, &node->hmap_node); > - json_destroy(node->json); > + json_destroy_with_yield(node->json); > free(node); > } > } > @@ -576,6 +577,7 @@ ovsdb_monitor_add_change_set(struct ovsdb_monitor *dbmon, > > struct shash_node *node; > SHASH_FOR_EACH (node, &dbmon->tables) { > + cooperative_multitasking_yield(); Is this needed? I don't see any heavy operations in this loop and the number of tables is usually not that large. > struct ovsdb_monitor_table *mt = node->data; > if (!init_only || (mt->select & OJMS_INITIAL)) { > struct ovsdb_monitor_change_set_for_table *mcst = > @@ -1172,6 +1174,7 @@ ovsdb_monitor_compose_update( > struct ovsdb_monitor_table *mt = mcst->mt; > > HMAP_FOR_EACH_SAFE (row, hmap_node, &mcst->rows) { > + cooperative_multitasking_yield(); > struct json *row_json; > row_json = (*row_update)(mt, condition, OVSDB_MONITOR_ROW, row, > initial, changed, mcst->n_columns); There should be yield in ovsdb_monitor_compose_cond_change_update() as well. > @@ -1286,8 +1289,9 @@ ovsdb_monitor_get_update( > > /* Pre-serializing the object to avoid doing this > * for every client. */ > - json_serialized = json_serialized_object_create(json); > - json_destroy(json); > + json_serialized = > + json_serialized_object_create_with_yield(json); > + json_destroy_with_yield(json); > json = json_serialized; > } > ovsdb_monitor_json_cache_insert(dbmon, version, mcs, > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c > index e1d4b1125..84c4834dc 100644 > --- a/ovsdb/ovsdb-server.c > +++ b/ovsdb/ovsdb-server.c > @@ -24,6 +24,7 @@ > > #include "column.h" > #include "command-line.h" > +#include "cooperative-multitasking.h" > #include "daemon.h" > #include "dirs.h" > #include "dns-resolve.h" > @@ -65,6 +66,8 @@ > > VLOG_DEFINE_THIS_MODULE(ovsdb_server); > > +static struct hmap cooperative_multitasking_callbacks; > + > struct db { > char *filename; > struct ovsdb *db; > @@ -340,6 +343,7 @@ main(int argc, char *argv[]) > fatal_ignore_sigpipe(); > process_init(); > dns_resolve_init(true); > + cooperative_multitasking_init(&cooperative_multitasking_callbacks); > > bool active = false; > parse_options(argc, argv, &db_filenames, &remotes, &unixctl_path, > @@ -532,6 +536,7 @@ main(int argc, char *argv[]) > } > dns_resolve_destroy(); > perf_counters_destroy(); > + cooperative_multitasking_destroy(); > service_stop(); > return 0; > } > diff --git a/ovsdb/trigger.c b/ovsdb/trigger.c > index 2a48ccc64..79360d359 100644 > --- a/ovsdb/trigger.c > +++ b/ovsdb/trigger.c > @@ -20,6 +20,7 @@ > #include <limits.h> > #include <string.h> > > +#include "cooperative-multitasking.h" > #include "file.h" > #include "openvswitch/json.h" > #include "jsonrpc.h" > @@ -181,6 +182,7 @@ ovsdb_trigger_run(struct ovsdb *db, long long int now) > bool disconnect_all = false; > > LIST_FOR_EACH_SAFE (t, node, &db->triggers) { > + cooperative_multitasking_yield(); An empty line would be nice. > if (run_triggers > || now - t->created >= t->timeout_msec > || t->progress || t->txn_forward) { > @@ -202,6 +204,7 @@ ovsdb_trigger_wait(struct ovsdb *db, long long int now) > struct ovsdb_trigger *t; > > LIST_FOR_EACH (t, node, &db->triggers) { > + cooperative_multitasking_yield(); Waiting is not a heavy operation. And we shouldn't have millions of active triggers. So, this yield is likely unnecessary. > if (t->created < LLONG_MAX - t->timeout_msec) { > long long int t_deadline = t->created + t->timeout_msec; > if (deadline > t_deadline) {
On Fri, Jan 12, 2024 at 1:40 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 1/10/24 20:29, Frode Nordahl wrote: > > Initialize the cooperative multitasking module for the > > ovsdb-server. > > > > The server side schema conversion process used for storage > > engines such as RAFT is time consuming, yield during > > processing. > > > > After the schema conversion is done, the processing of JSON-RPC > > sessions and OVSDB monitors for reconnecting clients can > > overrun the configured election timer. > > > > The JSON serialization of the database contents has been > > identified as one of the primary offenders. Make use of > > yielding versions of JSON object create and destroy functions > > to mitigate. > > > > This series has been tested by checking success of schema > > conversion, ensuring no involuntary leader change occurs with > > election timer configurations as low as 750 msec, on a 75MB > > database with ~ 100 connected clients as produced by the > > ovn-heater ocp-120-density-light test scenario. > > Nice! > > > > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > > --- > > NEWS | 8 ++++++++ > > ovsdb/file.c | 2 ++ > > ovsdb/jsonrpc-server.c | 2 ++ > > ovsdb/monitor.c | 12 ++++++++---- > > ovsdb/ovsdb-server.c | 5 +++++ > > ovsdb/trigger.c | 3 +++ > > 6 files changed, 28 insertions(+), 4 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index 270ed6673..643fddc98 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -6,6 +6,9 @@ Post-v3.2.0 > > from older version is supported but it may trigger more leader elections > > during the process, and error logs complaining unrecognized fields may > > be observed on old nodes. > > + * Make use of cooperative multitasking to ensure stable maintenance of > > + RAFT cluster during long running processing such as online schema > > + conversion. > > - OpenFlow: > > * NXT_CT_FLUSH extension is updated to support flushing connections > > based on mark and labels. 'ct-flush' command of ovs-ofctl updated > > @@ -36,6 +39,11 @@ Post-v3.2.0 > > The existing behaviour is maintained and a non key:value pair value > > will be applied to all other PMD thread cores.'pmd-sleep-show' is > > updated to show the maximum sleep for each PMD thread core. > > + - lib: > > + * Introduce cooperative multitasking module which allow us to interleave > > + important processing with long running tasks while avoiding the > > + additional resource consumption of threads and complexity of > > + asynchronous state machines. > > NEWS are mostly for user-visible changes. They don't need > to know about a new internal library. Entry in the OVSDB > section is enough. Ack. > > > > > > v3.2.0 - 17 Aug 2023 > > diff --git a/ovsdb/file.c b/ovsdb/file.c > > index 8bd1d4af3..fb4cbfb77 100644 > > --- a/ovsdb/file.c > > +++ b/ovsdb/file.c > > @@ -23,6 +23,7 @@ > > > > #include "bitmap.h" > > #include "column.h" > > +#include "cooperative-multitasking.h" > > #include "log.h" > > #include "openvswitch/json.h" > > #include "lockfile.h" > > @@ -308,6 +309,7 @@ ovsdb_convert_table(struct ovsdb_txn *txn, > > } > > > > HMAP_FOR_EACH (src_row, hmap_node, &src_table->rows) { > > + cooperative_multitasking_yield(); > > Please, move this after the empty line below variable declaration. Ack. > > struct ovsdb_row *dst_row = ovsdb_row_create(dst_table); > > *ovsdb_row_get_uuid_rw(dst_row) = *ovsdb_row_get_uuid(src_row); > > > > diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c > > index 45f7c8038..f08b5a946 100644 > > --- a/ovsdb/jsonrpc-server.c > > +++ b/ovsdb/jsonrpc-server.c > > @@ -21,6 +21,7 @@ > > > > #include "bitmap.h" > > #include "column.h" > > +#include "cooperative-multitasking.h" > > #include "openvswitch/dynamic-string.h" > > #include "monitor.h" > > #include "openvswitch/json.h" > > @@ -600,6 +601,7 @@ ovsdb_jsonrpc_session_run_all(struct ovsdb_jsonrpc_remote *remote) > > struct ovsdb_jsonrpc_session *s; > > > > LIST_FOR_EACH_SAFE (s, node, &remote->sessions) { > > + cooperative_multitasking_yield(); > > An empty line here would be nice. Ack. > > int error = ovsdb_jsonrpc_session_run(s); > > if (error) { > > ovsdb_jsonrpc_session_close(s); > > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c > > index d1e466faa..93cc66b76 100644 > > --- a/ovsdb/monitor.c > > +++ b/ovsdb/monitor.c > > @@ -20,6 +20,7 @@ > > > > #include "bitmap.h" > > #include "column.h" > > +#include "cooperative-multitasking.h" > > #include "openvswitch/dynamic-string.h" > > #include "openvswitch/json.h" > > #include "jsonrpc.h" > > @@ -262,7 +263,7 @@ ovsdb_monitor_json_cache_flush(struct ovsdb_monitor *dbmon) > > struct ovsdb_monitor_json_cache_node *node; > > > > HMAP_FOR_EACH_POP(node, hmap_node, &dbmon->json_cache) { > > - json_destroy(node->json); > > + json_destroy_with_yield(node->json); > > free(node); > > } > > } > > @@ -278,7 +279,7 @@ ovsdb_monitor_json_cache_destroy(struct ovsdb_monitor *dbmon, > > = ovsdb_monitor_json_cache_search(dbmon, v, change_set); > > if (node) { > > hmap_remove(&dbmon->json_cache, &node->hmap_node); > > - json_destroy(node->json); > > + json_destroy_with_yield(node->json); > > free(node); > > } > > } > > @@ -576,6 +577,7 @@ ovsdb_monitor_add_change_set(struct ovsdb_monitor *dbmon, > > > > struct shash_node *node; > > SHASH_FOR_EACH (node, &dbmon->tables) { > > + cooperative_multitasking_yield(); > > Is this needed? I don't see any heavy operations in this loop > and the number of tables is usually not that large. I checked logs and it had not fired here for successful runs, so I'll drop it. Thx! > > struct ovsdb_monitor_table *mt = node->data; > > if (!init_only || (mt->select & OJMS_INITIAL)) { > > struct ovsdb_monitor_change_set_for_table *mcst = > > @@ -1172,6 +1174,7 @@ ovsdb_monitor_compose_update( > > struct ovsdb_monitor_table *mt = mcst->mt; > > > > HMAP_FOR_EACH_SAFE (row, hmap_node, &mcst->rows) { > > + cooperative_multitasking_yield(); > > struct json *row_json; > > row_json = (*row_update)(mt, condition, OVSDB_MONITOR_ROW, row, > > initial, changed, mcst->n_columns); > > There should be yield in ovsdb_monitor_compose_cond_change_update() > as well. Thanks! > > @@ -1286,8 +1289,9 @@ ovsdb_monitor_get_update( > > > > /* Pre-serializing the object to avoid doing this > > * for every client. */ > > - json_serialized = json_serialized_object_create(json); > > - json_destroy(json); > > + json_serialized = > > + json_serialized_object_create_with_yield(json); > > + json_destroy_with_yield(json); > > json = json_serialized; > > } > > ovsdb_monitor_json_cache_insert(dbmon, version, mcs, > > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c > > index e1d4b1125..84c4834dc 100644 > > --- a/ovsdb/ovsdb-server.c > > +++ b/ovsdb/ovsdb-server.c > > @@ -24,6 +24,7 @@ > > > > #include "column.h" > > #include "command-line.h" > > +#include "cooperative-multitasking.h" > > #include "daemon.h" > > #include "dirs.h" > > #include "dns-resolve.h" > > @@ -65,6 +66,8 @@ > > > > VLOG_DEFINE_THIS_MODULE(ovsdb_server); > > > > +static struct hmap cooperative_multitasking_callbacks; > > + > > struct db { > > char *filename; > > struct ovsdb *db; > > @@ -340,6 +343,7 @@ main(int argc, char *argv[]) > > fatal_ignore_sigpipe(); > > process_init(); > > dns_resolve_init(true); > > + cooperative_multitasking_init(&cooperative_multitasking_callbacks); > > > > bool active = false; > > parse_options(argc, argv, &db_filenames, &remotes, &unixctl_path, > > @@ -532,6 +536,7 @@ main(int argc, char *argv[]) > > } > > dns_resolve_destroy(); > > perf_counters_destroy(); > > + cooperative_multitasking_destroy(); > > service_stop(); > > return 0; > > } > > diff --git a/ovsdb/trigger.c b/ovsdb/trigger.c > > index 2a48ccc64..79360d359 100644 > > --- a/ovsdb/trigger.c > > +++ b/ovsdb/trigger.c > > @@ -20,6 +20,7 @@ > > #include <limits.h> > > #include <string.h> > > > > +#include "cooperative-multitasking.h" > > #include "file.h" > > #include "openvswitch/json.h" > > #include "jsonrpc.h" > > @@ -181,6 +182,7 @@ ovsdb_trigger_run(struct ovsdb *db, long long int now) > > bool disconnect_all = false; > > > > LIST_FOR_EACH_SAFE (t, node, &db->triggers) { > > + cooperative_multitasking_yield(); > > An empty line would be nice. Ack. > > if (run_triggers > > || now - t->created >= t->timeout_msec > > || t->progress || t->txn_forward) { > > @@ -202,6 +204,7 @@ ovsdb_trigger_wait(struct ovsdb *db, long long int now) > > struct ovsdb_trigger *t; > > > > LIST_FOR_EACH (t, node, &db->triggers) { > > + cooperative_multitasking_yield(); > > Waiting is not a heavy operation. And we shouldn't have millions > of active triggers. So, this yield is likely unnecessary. Ack. > > if (t->created < LLONG_MAX - t->timeout_msec) { > > long long int t_deadline = t->created + t->timeout_msec; > > if (deadline > t_deadline) { >
diff --git a/NEWS b/NEWS index 270ed6673..643fddc98 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,9 @@ Post-v3.2.0 from older version is supported but it may trigger more leader elections during the process, and error logs complaining unrecognized fields may be observed on old nodes. + * Make use of cooperative multitasking to ensure stable maintenance of + RAFT cluster during long running processing such as online schema + conversion. - OpenFlow: * NXT_CT_FLUSH extension is updated to support flushing connections based on mark and labels. 'ct-flush' command of ovs-ofctl updated @@ -36,6 +39,11 @@ Post-v3.2.0 The existing behaviour is maintained and a non key:value pair value will be applied to all other PMD thread cores.'pmd-sleep-show' is updated to show the maximum sleep for each PMD thread core. + - lib: + * Introduce cooperative multitasking module which allow us to interleave + important processing with long running tasks while avoiding the + additional resource consumption of threads and complexity of + asynchronous state machines. v3.2.0 - 17 Aug 2023 diff --git a/ovsdb/file.c b/ovsdb/file.c index 8bd1d4af3..fb4cbfb77 100644 --- a/ovsdb/file.c +++ b/ovsdb/file.c @@ -23,6 +23,7 @@ #include "bitmap.h" #include "column.h" +#include "cooperative-multitasking.h" #include "log.h" #include "openvswitch/json.h" #include "lockfile.h" @@ -308,6 +309,7 @@ ovsdb_convert_table(struct ovsdb_txn *txn, } HMAP_FOR_EACH (src_row, hmap_node, &src_table->rows) { + cooperative_multitasking_yield(); struct ovsdb_row *dst_row = ovsdb_row_create(dst_table); *ovsdb_row_get_uuid_rw(dst_row) = *ovsdb_row_get_uuid(src_row); diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c index 45f7c8038..f08b5a946 100644 --- a/ovsdb/jsonrpc-server.c +++ b/ovsdb/jsonrpc-server.c @@ -21,6 +21,7 @@ #include "bitmap.h" #include "column.h" +#include "cooperative-multitasking.h" #include "openvswitch/dynamic-string.h" #include "monitor.h" #include "openvswitch/json.h" @@ -600,6 +601,7 @@ ovsdb_jsonrpc_session_run_all(struct ovsdb_jsonrpc_remote *remote) struct ovsdb_jsonrpc_session *s; LIST_FOR_EACH_SAFE (s, node, &remote->sessions) { + cooperative_multitasking_yield(); int error = ovsdb_jsonrpc_session_run(s); if (error) { ovsdb_jsonrpc_session_close(s); diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c index d1e466faa..93cc66b76 100644 --- a/ovsdb/monitor.c +++ b/ovsdb/monitor.c @@ -20,6 +20,7 @@ #include "bitmap.h" #include "column.h" +#include "cooperative-multitasking.h" #include "openvswitch/dynamic-string.h" #include "openvswitch/json.h" #include "jsonrpc.h" @@ -262,7 +263,7 @@ ovsdb_monitor_json_cache_flush(struct ovsdb_monitor *dbmon) struct ovsdb_monitor_json_cache_node *node; HMAP_FOR_EACH_POP(node, hmap_node, &dbmon->json_cache) { - json_destroy(node->json); + json_destroy_with_yield(node->json); free(node); } } @@ -278,7 +279,7 @@ ovsdb_monitor_json_cache_destroy(struct ovsdb_monitor *dbmon, = ovsdb_monitor_json_cache_search(dbmon, v, change_set); if (node) { hmap_remove(&dbmon->json_cache, &node->hmap_node); - json_destroy(node->json); + json_destroy_with_yield(node->json); free(node); } } @@ -576,6 +577,7 @@ ovsdb_monitor_add_change_set(struct ovsdb_monitor *dbmon, struct shash_node *node; SHASH_FOR_EACH (node, &dbmon->tables) { + cooperative_multitasking_yield(); struct ovsdb_monitor_table *mt = node->data; if (!init_only || (mt->select & OJMS_INITIAL)) { struct ovsdb_monitor_change_set_for_table *mcst = @@ -1172,6 +1174,7 @@ ovsdb_monitor_compose_update( struct ovsdb_monitor_table *mt = mcst->mt; HMAP_FOR_EACH_SAFE (row, hmap_node, &mcst->rows) { + cooperative_multitasking_yield(); struct json *row_json; row_json = (*row_update)(mt, condition, OVSDB_MONITOR_ROW, row, initial, changed, mcst->n_columns); @@ -1286,8 +1289,9 @@ ovsdb_monitor_get_update( /* Pre-serializing the object to avoid doing this * for every client. */ - json_serialized = json_serialized_object_create(json); - json_destroy(json); + json_serialized = + json_serialized_object_create_with_yield(json); + json_destroy_with_yield(json); json = json_serialized; } ovsdb_monitor_json_cache_insert(dbmon, version, mcs, diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index e1d4b1125..84c4834dc 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -24,6 +24,7 @@ #include "column.h" #include "command-line.h" +#include "cooperative-multitasking.h" #include "daemon.h" #include "dirs.h" #include "dns-resolve.h" @@ -65,6 +66,8 @@ VLOG_DEFINE_THIS_MODULE(ovsdb_server); +static struct hmap cooperative_multitasking_callbacks; + struct db { char *filename; struct ovsdb *db; @@ -340,6 +343,7 @@ main(int argc, char *argv[]) fatal_ignore_sigpipe(); process_init(); dns_resolve_init(true); + cooperative_multitasking_init(&cooperative_multitasking_callbacks); bool active = false; parse_options(argc, argv, &db_filenames, &remotes, &unixctl_path, @@ -532,6 +536,7 @@ main(int argc, char *argv[]) } dns_resolve_destroy(); perf_counters_destroy(); + cooperative_multitasking_destroy(); service_stop(); return 0; } diff --git a/ovsdb/trigger.c b/ovsdb/trigger.c index 2a48ccc64..79360d359 100644 --- a/ovsdb/trigger.c +++ b/ovsdb/trigger.c @@ -20,6 +20,7 @@ #include <limits.h> #include <string.h> +#include "cooperative-multitasking.h" #include "file.h" #include "openvswitch/json.h" #include "jsonrpc.h" @@ -181,6 +182,7 @@ ovsdb_trigger_run(struct ovsdb *db, long long int now) bool disconnect_all = false; LIST_FOR_EACH_SAFE (t, node, &db->triggers) { + cooperative_multitasking_yield(); if (run_triggers || now - t->created >= t->timeout_msec || t->progress || t->txn_forward) { @@ -202,6 +204,7 @@ ovsdb_trigger_wait(struct ovsdb *db, long long int now) struct ovsdb_trigger *t; LIST_FOR_EACH (t, node, &db->triggers) { + cooperative_multitasking_yield(); if (t->created < LLONG_MAX - t->timeout_msec) { long long int t_deadline = t->created + t->timeout_msec; if (deadline > t_deadline) {
Initialize the cooperative multitasking module for the ovsdb-server. The server side schema conversion process used for storage engines such as RAFT is time consuming, yield during processing. After the schema conversion is done, the processing of JSON-RPC sessions and OVSDB monitors for reconnecting clients can overrun the configured election timer. The JSON serialization of the database contents has been identified as one of the primary offenders. Make use of yielding versions of JSON object create and destroy functions to mitigate. This series has been tested by checking success of schema conversion, ensuring no involuntary leader change occurs with election timer configurations as low as 750 msec, on a 75MB database with ~ 100 connected clients as produced by the ovn-heater ocp-120-density-light test scenario. Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> --- NEWS | 8 ++++++++ ovsdb/file.c | 2 ++ ovsdb/jsonrpc-server.c | 2 ++ ovsdb/monitor.c | 12 ++++++++---- ovsdb/ovsdb-server.c | 5 +++++ ovsdb/trigger.c | 3 +++ 6 files changed, 28 insertions(+), 4 deletions(-)