<HTML><BODY><div>Thanks for the patch!<br>LGTM.</div><div> </div><div> </div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Четверг, 20 августа 2020, 0:35 +03:00 от Cyrill Gorcunov <gorcunov@gmail.com>:<br> <div id=""><div class="js-helper js-readmsg-msg"><style type="text/css"></style><div><div id="style_15978729492041660360_BODY">When we need to write CONFIRM or ROLLBACK message (which is<br>a binary record in msgpack format) into a journal we use txn code<br>to allocate a new transaction, encode there a message and pass it<br>to walk the long txn path before it hit the journal. This is not<br>only resource wasting but also somehow strange from architectural<br>point of view.<br><br>Instead lets encode a record on the stack and write it to the journal<br>directly.<br><br>Part-of #5129<br><br>Signed-off-by: Cyrill Gorcunov <<a href="/compose?To=gorcunov@gmail.com">gorcunov@gmail.com</a>><br>---<br> src/box/txn_limbo.c | 66 +++++++++++++++++++++++----------------------<br> 1 file changed, 34 insertions(+), 32 deletions(-)<br><br>diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c<br>index e458dad75..4b90d7fa5 100644<br>--- a/src/box/txn_limbo.c<br>+++ b/src/box/txn_limbo.c<br>@@ -32,6 +32,7 @@<br> #include "txn_limbo.h"<br> #include "replication.h"<br> #include "iproto_constants.h"<br>+#include "journal.h"<br> <br> struct txn_limbo txn_limbo;<br> <br>@@ -272,6 +273,17 @@ txn_limbo_wait_complete(struct txn_limbo *limbo, struct txn_limbo_entry *entry)<br> return 0;<br> }<br> <br>+/**<br>+ * A callback for synchronous write: txn_limbo_write_synchro fiber<br>+ * waiting to proceed once a record is written to WAL.<br>+ */<br>+static void<br>+txn_limbo_write_cb(struct journal_entry *entry)<br>+{<br>+ assert(entry->complete_data != NULL);<br>+ fiber_wakeup(entry->complete_data);<br>+}<br>+<br> static void<br> txn_limbo_write_synchro(struct txn_limbo *limbo, uint32_t type, int64_t lsn)<br> {<br>@@ -285,46 +297,36 @@ txn_limbo_write_synchro(struct txn_limbo *limbo, uint32_t type, int64_t lsn)<br> <br> /*<br> * This is a synchronous commit so we can<br>- * use body and row allocated on a stack.<br>+ * allocate everything on a stack.<br> */<br> struct synchro_body_bin body;<br> struct xrow_header row;<br>- struct request request = {<br>- .header = &row,<br>- };<br>+ char buf[sizeof(struct journal_entry) +<br>+ sizeof(struct xrow_header *)];<br> <br>- struct txn *txn = txn_begin();<br>- if (txn == NULL)<br>- goto rollback;<br>+ struct journal_entry *entry = (struct journal_entry *)buf;<br>+ entry->rows[0] = &row;<br> <br> xrow_encode_synchro(&row, &body, &req);<br> <br>- /*<br>- * This is not really a transaction. It just uses txn API<br>- * to put the data into WAL. And obviously it should not<br>- * go to the limbo and block on the very same sync<br>- * transaction which it tries to confirm now.<br>- */<br>- txn_set_flag(txn, TXN_FORCE_ASYNC);<br>-<br>- if (txn_begin_stmt(txn, NULL) != 0)<br>- goto rollback;<br>- if (txn_commit_stmt(txn, &request) != 0)<br>- goto rollback;<br>- if (txn_commit(txn) != 0)<br>- goto rollback;<br>- return;<br>+ journal_entry_create(entry, 1, xrow_approx_len(&row),<br>+ txn_limbo_write_cb, fiber());<br> <br>-rollback:<br>- /*<br>- * XXX: the stub is supposed to be removed once it is defined what to do<br>- * when a synchro request WAL write fails. One of the possible<br>- * solutions: log the error, keep the limbo queue as is and probably put<br>- * in rollback mode. Then provide a hook to call manually when WAL<br>- * problems are fixed. Or retry automatically with some period.<br>- */<br>- panic("Could not write a synchro request to WAL: lsn = %lld, type = "<br>- "%s\n", lsn, iproto_type_name(type));<br>+ if (journal_write(entry) != 0 || entry->res < 0) {<br>+ diag_set(ClientError, ER_WAL_IO);<br>+ diag_log();<br>+ /*<br>+ * XXX: the stub is supposed to be removed once it is defined<br>+ * what to do when a synchro request WAL write fails. One of<br>+ * the possible solutions: log the error, keep the limbo<br>+ * queue as is and probably put in rollback mode. Then<br>+ * provide a hook to call manually when WAL problems are fixed.<br>+ * Or retry automatically with some period.<br>+ */<br>+ panic("Could not write a synchro request to WAL: "<br>+ "lsn = %lld, type = %s\n", lsn,<br>+ type == IPROTO_CONFIRM ? "CONFIRM" : "ROLLBACK");<br>+ }<br> }<br> <br> /**<br>--<br>2.26.2</div></div></div></div></blockquote><div><div> <div> </div><div><div>--<br>Serge Petrenko<span id="cke_bm_403E" style="display: none;"> </span></div></div></div></div><div> </div></BODY></HTML>