Node is unnecessarily locked during transmission of trace to Dispatch

[moralismercatus]

Look at https://github.com/SVL-PSU/crete-dev/blob/71ab5f8f3c6116e1024219d6b93190102c956584/lib/include/crete/cluster/node_driver.h#L302:L323

Notice that lock's mutex is released upon function scope exit. It is held during transmission of the trace. I don't believe this is necessary. Rather, I believe:

 auto lock = node.acquire();
 auto trace = lock->pop_trace();

Should be:

 auto trace = node.acquire()->pop_trace();

Thus node is immediately released upon completion of the statement.

[likebreath]

@moralismercatus This change was introduced at commit https://github.com/SVL-PSU/crete-dev/commit/9d5e8acb6de068a3b0cf78faf5f19f480cd2c8dc for fixing issue #6. Please let me know your thoughts.

[moralismercatus]

@likebreath I vaguely remember issue #6 now. I understand the reasoning behind the change. I only happened upon it again while reviewing the flows for other slowdowns.

I'm not sure how pressing an issue this is presently, but, in general, I think a better solution is in order. Especially for very large traces that take a considerable time to archive and transmit, the whole VM node essentially sits idly by and waits for the transmission to complete.

[likebreath]

I'm not sure how pressing an issue this is presently, but, in general, I think a better solution is in order. Especially for very large traces that take a considerable time to archive and transmit, the whole VM node essentially sits idly by and waits for the transmission to complete.

Totally agree. I think it will be a major slow down when the trace size is large. You may want to look at the patch I did for issue #32 to alleviate the slowdown when trace size is large.

We need to have a patch to fix the slowdown without re-introducing the crash in issue #6.

[moralismercatus]

Thanks for the reminder on these issues. I'll ponder these issues and see if I can't come up with a PR or two.