On Fri, Jan 12, 2024 at 5:20 PM Ilya Maximets <[email protected]> wrote: > > On 1/12/24 01:27, Ilya Maximets wrote: > > On 1/10/24 20:29, Frode Nordahl wrote: > >> During testing of the implementation of cooperative multitasking > >> in the ovsdb-server we noticed that the very first yield being > >> called in the jsonrpc-server always fired. > >> > >> This indicates that the operations being after/before storage run > >> is taking too long as it currently is. > >> > >> Moving the storage run section to the top of the main loop lead > >> to successful results using less yield calls as documented in the > >> commit message of the next commit in this series. > > > > I don't really understand what exactly is taking so much time and > > why moving this part of the code helps. Could you elaborate? > > I think, I got it. In the current version of a patch set we have: > > main(): > 0. open_db() > raft_alloc() <-- register callback > read_db() <-- takes a lot of time > main_loop() > 1. run jsonrpc <-- yields and warns because read_db > already took a lot of time. > yield() > raft_run() <-- updates the callback > 2. storage_run() > raft_run() <-- updates the callback > > The solution proposed in this patch is to swap 1 and 2, > so we call raft_run() directly before we yield for the > first time. > > I suppose, my seggestion to not have _register() function would > solve that problem, because the callback will not be registered > until the first call to raft_run(). > > Is that correct?
Unfortunately, also when only having the _set() function the first yield always fires. Putting log lines before/after every major function call in the main_loop provides something like this: 2024-01-12T22:01:46.968Z|00204|ovsdb_server|DBG|HELLO 0 2024-01-12T22:01:46.968Z|00205|ovsdb_server|DBG|HELLO 1 2024-01-12T22:01:46.968Z|00206|ovsdb_server|DBG|HELLO 2 2024-01-12T22:01:46.968Z|00207|ovsdb_server|DBG|HELLO 3 2024-01-12T22:01:46.968Z|00208|ovsdb_server|DBG|HELLO 4 2024-01-12T22:01:46.968Z|00209|ovsdb_server|DBG|HELLO 5 2024-01-12T22:01:46.968Z|00210|ovsdb_server|DBG|HELLO 6 2024-01-12T22:01:47.134Z|00211|ovsdb_server|DBG|HELLO 0 2024-01-12T22:01:47.134Z|00212|ovsdb_server|DBG|HELLO 1 2024-01-12T22:01:47.134Z|00213|ovsdb_server|DBG|HELLO 2 2024-01-12T22:01:47.134Z|00214|ovsdb_server|DBG|HELLO 3 2024-01-12T22:01:47.134Z|00215|ovsdb_server|DBG|HELLO 4 2024-01-12T22:01:47.134Z|00216|ovsdb_server|DBG|HELLO 5 2024-01-12T22:01:47.135Z|00217|cooperative_multitasking|DBG|ovsdb/jsonrpc-server.c:604: yield for raft_run(0x560b47e225e0): elapsed(167) >= threshold(166), overrun: 1 So I actually wonder if the wait timers set up in raft_wait() are too optimistic? Then again, those timers can't ever take into account the number of clients serviced by for example the call to ovsdb_jsonrpc_server_run(). I know that reordering the whole thing is a bit drastic, and I kind of regret proposing that. Would an alternative be to slip in a yield at the top of the main_loop function or a SHASH_FOR_EACH_SAFE (node, all_dbs) that just does ovsdb_storage_run()? -- Frode Nordahl > > > > Best regards, Ilya Maximets. > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
