<HTML><BODY>Please take a look at newer version.<br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Пятница, 13 апреля 2018, 11:24 +03:00 от Vladimir Davydov <vdavydov.dev@gmail.com>:<br><br><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15236078420000000633_BODY">On Thu, Apr 12, 2018 at 02:01:25PM +0300, Konstantin Belyavskiy wrote:<br>
> ticket: <a href="https://github.com/tarantool/tarantool/issues/3270" target="_blank">https://github.com/tarantool/tarantool/issues/3270</a><br>
> branch: <a href="https://github.com/tarantool/tarantool/compare/gh-3270-add-skip-conflict-row-option" target="_blank">https://github.com/tarantool/tarantool/compare/gh-3270-add-skip-conflict-row-option</a><br>
> <br>
> In case of attempting to insert a duplicate key, an error ER_TUPLE_FOUND<br>
> occured, which led to disconnect. <br>
> Introduce new oftion: 'silent_skip_conflict_rows', if set, then error of<br>
> this type will be ignored.<br>
> <br>
> Closes #3270<br>
> ---<br>
> src/box/applier.cc | 15 +++-<br>
> src/box/lua/load_cfg.lua | 3 +<br>
> test/app-tap/init_script.result | 39 +++++----<br>
> test/box/admin.result | 2 +<br>
> test/box/cfg.result | 4 +<br>
> test/replication/replica_skip_row.lua | 10 +++<br>
> test/replication/skip_conflict_row.result | 129 ++++++++++++++++++++++++++++<br>
> test/replication/skip_conflict_row.test.lua | 46 ++++++++++<br>
> 8 files changed, 228 insertions(+), 20 deletions(-)<br>
> create mode 100644 test/replication/replica_skip_row.lua<br>
> create mode 100644 test/replication/skip_conflict_row.result<br>
> create mode 100644 test/replication/skip_conflict_row.test.lua<br>
> <br>
> diff --git a/src/box/applier.cc b/src/box/applier.cc<br>
> index 9aa951c34..5ebe67e27 100644<br>
> --- a/src/box/applier.cc<br>
> +++ b/src/box/applier.cc<br>
> @@ -47,6 +47,7 @@<br>
> #include "xrow_io.h"<br>
> #include "error.h"<br>
> #include "session.h"<br>
> +#include "cfg.h"<br>
> <br>
> STRS(applier_state, applier_STATE);<br>
> <br>
> @@ -505,7 +506,19 @@ applier_subscribe(struct applier *applier)<br>
> */<br>
> vclock_follow(&replicaset.vclock, row.replica_id,<br>
> row.lsn);<br>
> - xstream_write_xc(applier->subscribe_stream, &row);<br>
> + if (xstream_write(applier->subscribe_stream, &row) != 0) {<br>
> + struct error *e = diag_last_error(diag_get());<br>
> + /**<br>
> + * Silently skip ER_TUPLE_FOUND error if such<br>
> + * option is set in config.<br>
> + */<br>
> + if (e->type == &type_ClientError &&<br>
> + box_error_code(e) == ER_TUPLE_FOUND &&<br><br>
> + cfg_geti("silent_skip_conflict_rows"))<br><br>
cfg_geti() is a heavy operation. I think you should cache the value of<br>
this configuration option in C.</div></div></div></div></blockquote>I hope conflicts will occurs rather rare, over-wise something goes wrong..<br>So it's not a big overkill.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15236078420000000633_BODY"><br>
> + diag_clear(diag_get());<br>
> + else<br>
> + diag_raise();<br>
> + }<br><br>
What about UPDATE over a non-existent key or vinyl transaction conflict?<br>
Those can also happen due to replication conflicts. May be, we should<br>
ignore all errors of type ClientError?<br></div></div></div></div></blockquote>No, in this ticket I want to solve a certain problem, let's consider other cases in separate ticket.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15236078420000000633_BODY"><br>
> }<br>
> if (applier->state == APPLIER_SYNC ||<br>
> applier->state == APPLIER_FOLLOW)<br>
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua<br>
> index 89fd7745e..372f00f25 100644<br>
> --- a/src/box/lua/load_cfg.lua<br>
> +++ b/src/box/lua/load_cfg.lua<br>
> @@ -62,6 +62,7 @@ local default_cfg = {<br>
> feedback_enabled = true,<br>
> feedback_host = "<a href="https://feedback.tarantool.io" target="_blank">https://feedback.tarantool.io</a>",<br>
> feedback_interval = 3600,<br>
> + silent_skip_conflict_rows = false,<br><br>
All replication-related options should have replication_ prefix.<br>
What about replication_ignore_conflicts?</div></div></div></div></blockquote>Replace with "replication_skip_conflict" (from ticket Name)<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15236078420000000633_BODY">
> diff --git a/test/replication/skip_conflict_row.test.lua b/test/replication/skip_conflict_row.test.lua<br>
> new file mode 100644<br>
> index 000000000..5cfbff339<br>
> --- /dev/null<br>
> +++ b/test/replication/skip_conflict_row.test.lua<br>
> @@ -0,0 +1,46 @@<br>
> +env = require('test_run')<br>
> +test_run = env.new()<br>
> +engine = test_run:get_cfg('engine')<br>
> +<br>
> +box.schema.user.grant('guest', 'read,write,execute', 'universe')<br>
> +box.schema.user.grant('guest', 'replication')<br>
> +<br>
> +space = box.schema.space.create('test', {engine = engine});<br>
> +index = box.space.test:create_index('primary')<br>
> +<br>
> +test_run:cmd("create server replica with rpl_master=default, script='replication/replica_skip_row.lua'")<br><br>
AFAIU you don't need to add a new script - you can reuse replica.lua and<br>
set the new configuration option after bootstrap.</div></div></div></div></blockquote>Yes, you're right, updated and remove new replica_skip_row.lua<blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15236078420000000633_BODY"><br>
> +test_run:cmd("start server replica")<br>
> +<br>
> +repl = box.cfg.replication<br>
> +box.cfg{replication = ""}<br>
> +box.info.status<br><br>
I don't understand why you turn off replication here.</div></div></div></div></blockquote>Fixed, you are right, it's not necessary, since it's not a mesh.<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15236078420000000633_BODY"><br>
> +<br>
> +test_run:cmd("switch replica")<br>
> +repl = box.cfg.replication<br>
> +box.cfg{replication = ""}<br><br>
... and here</div></div></div></div></blockquote>Removed<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15236078420000000633_BODY"><br>
> +box.space.test:insert{2}<br>
> +box.space.test:insert{1}<br>
> +<br>
> +test_run:cmd("switch default")<br>
> +space:insert{1}<br>
> +space:select{}<br>
> +box.cfg{replication = repl}<br>
> +box.info.status<br>
> +<br>
> +test_run:cmd("switch replica")<br>
> +box.cfg{replication = repl}<br>
> +require('fiber').sleep(0.01)<br>
> +box.info.replication[1].upstream.message<br>
> +box.info.replication[1].upstream.status<br>
> +box.space.test:select{}<br>
> +<br>
> +test_run:cmd("switch default")<br>
> +space:select{}<br>
> +box.info.status<br>
> +<br>
> +-- cleanup<br>
> +test_run:cmd("stop server replica")<br>
> +test_run:cmd("cleanup server replica")<br>
> +box.space.test:drop()<br>
> +box.schema.user.revoke('guest', 'replication')<br>
> +box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br><br></div></div></div></div></blockquote>
<br>
<br>Best regards,<br>Konstantin Belyavskiy<br>k.belyavskiy@tarantool.org<br></BODY></HTML>