<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><blockquote type="cite" class=""><blockquote type="cite" class="">Instead, I propose a simple one line change. Could you please describe<br class="">the reproduction of the probable problem step by step?</blockquote></blockquote><div class=""><br class=""></div>And what’s the problem with the current Leonid’s solution after all?<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On 11 Dec 2020, at 15:51, Nikita Pettik <<a href="mailto:korablev@tarantool.org" class="">korablev@tarantool.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">On 11 Dec 02:08, Leonid Vasiliev wrote:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">Hi! Thank you for review.<br class=""><br class="">On 10.12.2020 19:39, Sergey Ostanevich wrote:<br class=""><blockquote type="cite" class="">Thanks for the patch!<br class=""><br class="">I tend to the 1st alternative, although the code using the name generated<br class="">is hairy. I believe the same resolution as for the first part: if we’re<br class="">in a rush - LGTM, better solution is desirable otherwise.<br class=""></blockquote><br class="">As I wrote in the commit message, I think using `O_TMPFILE` or<br class="">`tmpfile()` is the best solution to work with temporary files.<br class="">But we already have the code in which the filename is passed from one<br class="">function to another, and some logic over all this. So, replacing the use of<br class="">a named file with a real temporary file will lead to refactoring,<br class="">changing some logic and increasing the number of differences with<br class="">SQLite. This can lead to degradation and complication of the review.<br class=""></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">Anyway, there's already huge difference between our codebases, so</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">don't worry about incompatibility between SQLite and Tarantool.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">If you don't want fix this problem now, could you please open issue</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">and describe problem there (copy-paste alternatives you suggested</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">in the previous letter)?</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">Instead, I propose a simple one line change. Could you please describe<br class="">the reproduction of the probable problem step by step?<br class=""><br class=""><blockquote type="cite" class="">Sergos<br class=""><br class=""><br class=""><blockquote type="cite" class="">On 8 Dec 2020, at 22:59, Leonid Vasiliev <<a href="mailto:lvasiliev@tarantool.org" class="">lvasiliev@tarantool.org</a>> wrote:<br class=""><br class="">The bug was consisted in fail when working with temporary files<br class="">created by VDBE to sort large result of a `SELECT` statement with<br class="">`ORDER BY`, `GROUP BY` clauses.<br class=""><br class="">Whats happen (step by step):<br class="">- We have two instances on one node (sharded cluster).<br class="">- A query is created that executes on both.<br class="">- The first instance creates the name of the temporary file and<br class=""> checks a file with such name on existence.<br class="">- The second instance creates the name of the temporary file<br class=""> (the same as in first instance) and checks a file with such name<br class=""> on existence.<br class="">- The first instance creates a file with the `SQL_OPEN_DELETEONCLOSE`<br class=""> flag.<br class="">- The second instance opens(try to open) the same file.<br class="">- The first instance closes (and removes) the temporary file.<br class="">- The second instance tries to work with the file and fails.<br class=""><br class="">Why did it happen:<br class="">The temporary file name format has a random part, but the random<br class="">generator uses a fixed seed.<br class=""><br class="">When it was decided to use a fixed seed:<br class="">32cb1ad298b2b55d8536a85bdfb3827c8c8739e1<br class=""><br class="">How the patch fixes the problem:<br class="">The patch injects the PID in the temporary file name format.<br class="">The generated name is unique for a single process (due to a random part)<br class="">and unique between processes (due to the PID part).<br class=""><br class="">Alternatives:<br class="">1) Use `O_TMPFILE` or `tmpfile()` (IMHO the best way to work with<br class=""> temporary files). In both cases, we need to update a significant<br class=""> part of the code, and some degradation can be added. It's hard to<br class=""> review.<br class="">2) Return a random seed for the generator. As far as I understand,<br class=""> we want to have good reproducible system behavior, in which case<br class=""> it's good to use a fixed seed.<br class="">3) Add reopening file with the flags `O_CREAT | O_EXCL` until we<br class=""> win the fight. Now we set such flags when opening a temporary file,<br class=""> but after that we try to open the file in `READONLY` mode and<br class=""> if ok - return the descriptor. This is strange logic for me and I<br class=""> don't want to add any aditional logic here. Also, such solution will<br class=""> add additional attempts to open the file.<br class=""><br class="">So, it look like such minimal changes will work fine and are simple<br class="">to review.<br class=""><br class="">Co-authored-by: Mergen Imeev<<a href="mailto:imeevma@gmail.com" class="">imeevma@gmail.com</a>><br class=""><br class="">Fixes #5537<br class="">---<br class="">src/box/sql/os_unix.c | 4 ++--<br class="">1 file changed, 2 insertions(+), 2 deletions(-)<br class=""><br class="">diff --git a/src/box/sql/os_unix.c b/src/box/sql/os_unix.c<br class="">index 557d709..ce415cb 100644<br class="">--- a/src/box/sql/os_unix.c<br class="">+++ b/src/box/sql/os_unix.c<br class="">@@ -1483,8 +1483,8 @@ unixGetTempname(int nBuf, char *zBuf)<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>assert(nBuf > 2);<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>zBuf[nBuf - 2] = 0;<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>sql_snprintf(nBuf, zBuf,<br class="">-<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span>"%s/" SQL_TEMP_FILE_PREFIX "%llx%c", zDir,<br class="">-<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span>r, 0);<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span>"%s/" SQL_TEMP_FILE_PREFIX "%ld_%llx%c", zDir,<br class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-converted-space"> </span>(long)randomnessPid, r, 0);<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>if (zBuf[nBuf - 2] != 0 || (iLimit++) > 10)<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span><span class="Apple-tab-span" style="white-space: pre;"> </span>return -1;<br class=""><span class="Apple-tab-span" style="white-space: pre;"> </span>} while (access(zBuf, 0) == 0);<br class="">--<span class="Apple-converted-space"> </span><br class="">2.7.4</blockquote></blockquote></blockquote></div></blockquote></div><br class=""></body></html>