* [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua @ 2020-04-16 19:07 imeevma 2020-04-16 19:09 ` Alexander Tikhonov 0 siblings, 1 reply; 14+ messages in thread From: imeevma @ 2020-04-16 19:07 UTC (permalink / raw) To: avtikhon, tarantool-patches Closes #4384 --- https://github.com/tarantool/tarantool/issues/4384 https://github.com/tarantool/tarantool/tree/imeevma/gh-4384-fix-test-func-recreate @ChangeLog - Fixed flaky test sql/func-recreate.test.lua (gh-4384). test/sql/func-recreate.result | 19 +++++++++++-------- test/sql/func-recreate.test.lua | 11 ++++++----- test/sql/suite.ini | 1 - 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result index 73fb03c..470ce5a 100644 --- a/test/sql/func-recreate.result +++ b/test/sql/func-recreate.result @@ -12,31 +12,34 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') fiber = require('fiber') --- ... -box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) +flag = true --- ... -ch = fiber.channel(1) +box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) --- ... -_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) +ch = fiber.channel(1) --- ... -fiber.sleep(0.1) +_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) --- ... -box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) +box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) --- - error: 'Failed to create function ''WAITFOR'': unable to create function due to active statements' ... +flag = false +--- +... ch:get() --- - metadata: - - name: WAITFOR(0.2) + - name: WAITFOR() type: integer rows: - - [0.2] + - [0] ... -box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) +box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) --- ... diff --git a/test/sql/func-recreate.test.lua b/test/sql/func-recreate.test.lua index 753e9ca..d482fa9 100644 --- a/test/sql/func-recreate.test.lua +++ b/test/sql/func-recreate.test.lua @@ -4,14 +4,15 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') -- Check errors during function create process fiber = require('fiber') -box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) +flag = true +box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) ch = fiber.channel(1) -_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) -fiber.sleep(0.1) +_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) -box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) +box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) +flag = false ch:get() -box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) +box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) diff --git a/test/sql/suite.ini b/test/sql/suite.ini index a8664c5..cc1d641 100644 --- a/test/sql/suite.ini +++ b/test/sql/suite.ini @@ -11,4 +11,3 @@ release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua disabled = sql-statN-index-drop.test.lua pretest_clean = True fragile = dll.test.lua ; gh-4427 - func-recreate.test.lua ; gh-4384 -- 2.7.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-16 19:07 [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua imeevma @ 2020-04-16 19:09 ` Alexander Tikhonov 2020-04-16 19:18 ` Nikita Pettik 0 siblings, 1 reply; 14+ messages in thread From: Alexander Tikhonov @ 2020-04-16 19:09 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches [-- Attachment #1: Type: text/plain, Size: 3841 bytes --] Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. >Четверг, 16 апреля 2020, 22:07 +03:00 от imeevma@tarantool.org: > >Closes #4384 >--- >https://github.com/tarantool/tarantool/issues/4384 >https://github.com/tarantool/tarantool/tree/imeevma/gh-4384-fix-test-func-recreate > >@ChangeLog > - Fixed flaky test sql/func-recreate.test.lua (gh-4384). > > test/sql/func-recreate.result | 19 +++++++++++-------- > test/sql/func-recreate.test.lua | 11 ++++++----- > test/sql/suite.ini | 1 - > 3 files changed, 17 insertions(+), 14 deletions(-) > >diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result >index 73fb03c..470ce5a 100644 >--- a/test/sql/func-recreate.result >+++ b/test/sql/func-recreate.result >@@ -12,31 +12,34 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') > fiber = require('fiber') > --- > ... >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) >+flag = true > --- > ... >-ch = fiber.channel(1) >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > --- > ... >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) >+ch = fiber.channel(1) > --- > ... >-fiber.sleep(0.1) >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) > --- > ... >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > --- > - error: 'Failed to create function ''WAITFOR'': unable to create function due to > active statements' > ... >+flag = false >+--- >+... > ch:get() > --- > - metadata: >- - name: WAITFOR(0.2) >+ - name: WAITFOR() > type: integer > rows: >- - [0.2] >+ - [0] > ... >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > --- > ... >diff --git a/test/sql/func-recreate.test.lua b/test/sql/func-recreate.test.lua >index 753e9ca..d482fa9 100644 >--- a/test/sql/func-recreate.test.lua >+++ b/test/sql/func-recreate.test.lua >@@ -4,14 +4,15 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') > > -- Check errors during function create process > fiber = require('fiber') >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) >+flag = true >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > ch = fiber.channel(1) > >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) >-fiber.sleep(0.1) >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) >+flag = false > ch:get() >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > >diff --git a/test/sql/suite.ini b/test/sql/suite.ini >index a8664c5..cc1d641 100644 >--- a/test/sql/suite.ini >+++ b/test/sql/suite.ini >@@ -11,4 +11,3 @@ release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua > disabled = sql-statN-index-drop.test.lua > pretest_clean = True > fragile = dll.test.lua ; gh-4427 >- func-recreate.test.lua ; gh-4384 >-- >2.7.4 > -- Alexander Tikhonov [-- Attachment #2: Type: text/html, Size: 4859 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-16 19:09 ` Alexander Tikhonov @ 2020-04-16 19:18 ` Nikita Pettik 2020-04-16 19:33 ` Alexander Tikhonov 2020-04-16 20:24 ` Mergen Imeev 0 siblings, 2 replies; 14+ messages in thread From: Nikita Pettik @ 2020-04-16 19:18 UTC (permalink / raw) To: Alexander Tikhonov; +Cc: tarantool-patches On 16 Apr 22:09, Alexander Tikhonov wrote: > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. > Did you verify that modified test still reproduces initial problem? I hope so, but ask just in case. > >Четверг, 16 апреля 2020, 22:07 +03:00 от imeevma@tarantool.org: > > > >Closes #4384 > >--- > >https://github.com/tarantool/tarantool/issues/4384 > >https://github.com/tarantool/tarantool/tree/imeevma/gh-4384-fix-test-func-recreate > > > >@ChangeLog > > - Fixed flaky test sql/func-recreate.test.lua (gh-4384). > > > > test/sql/func-recreate.result | 19 +++++++++++-------- > > test/sql/func-recreate.test.lua | 11 ++++++----- > > test/sql/suite.ini | 1 - > > 3 files changed, 17 insertions(+), 14 deletions(-) > > > >diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result > >index 73fb03c..470ce5a 100644 > >--- a/test/sql/func-recreate.result > >+++ b/test/sql/func-recreate.result > >@@ -12,31 +12,34 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') > > fiber = require('fiber') > > --- > > ... > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) > >+flag = true > > --- > > ... > >-ch = fiber.channel(1) > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > --- > > ... > >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) > >+ch = fiber.channel(1) > > --- > > ... > >-fiber.sleep(0.1) > >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) > > --- > > ... > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > --- > > - error: 'Failed to create function ''WAITFOR'': unable to create function due to > > active statements' > > ... > >+flag = false > >+--- > >+... > > ch:get() > > --- > > - metadata: > >- - name: WAITFOR(0.2) > >+ - name: WAITFOR() > > type: integer > > rows: > >- - [0.2] > >+ - [0] > > ... > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > --- > > ... > >diff --git a/test/sql/func-recreate.test.lua b/test/sql/func-recreate.test.lua > >index 753e9ca..d482fa9 100644 > >--- a/test/sql/func-recreate.test.lua > >+++ b/test/sql/func-recreate.test.lua > >@@ -4,14 +4,15 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') > > > > -- Check errors during function create process > > fiber = require('fiber') > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) > >+flag = true > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > ch = fiber.channel(1) > > > >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) > >-fiber.sleep(0.1) > >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > >+flag = false > > ch:get() > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > >diff --git a/test/sql/suite.ini b/test/sql/suite.ini > >index a8664c5..cc1d641 100644 > >--- a/test/sql/suite.ini > >+++ b/test/sql/suite.ini > >@@ -11,4 +11,3 @@ release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua > > disabled = sql-statN-index-drop.test.lua > > pretest_clean = True > > fragile = dll.test.lua ; gh-4427 > >- func-recreate.test.lua ; gh-4384 > >-- > >2.7.4 > > > > > -- > Alexander Tikhonov > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-16 19:18 ` Nikita Pettik @ 2020-04-16 19:33 ` Alexander Tikhonov 2020-04-16 20:24 ` Mergen Imeev 1 sibling, 0 replies; 14+ messages in thread From: Alexander Tikhonov @ 2020-04-16 19:33 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches [-- Attachment #1: Type: text/plain, Size: 4639 bytes --] >Четверг, 16 апреля 2020, 22:18 +03:00 от Nikita Pettik <korablev@tarantool.org>: > >On 16 Apr 22:09, Alexander Tikhonov wrote: >> >> Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. >> > >Did you verify that modified test still reproduces initial problem? >I hope so, but ask just in case. Sure, I’ve checked this test before the fix and reproduced the needed issue just before the fix. With the current fix flaky fail resolved and checking error in the test became stable. > >> >Четверг, 16 апреля 2020, 22:07 +03:00 от imeevma@tarantool.org: >> > >> >Closes #4384 >> >--- >> > https://github.com/tarantool/tarantool/issues/4384 >> > https://github.com/tarantool/tarantool/tree/imeevma/gh-4384-fix-test-func-recreate >> > >> >@ChangeLog >> > - Fixed flaky test sql/func-recreate.test.lua (gh-4384). >> > >> > test/sql/func-recreate.result | 19 +++++++++++-------- >> > test/sql/func-recreate.test.lua | 11 ++++++----- >> > test/sql/suite.ini | 1 - >> > 3 files changed, 17 insertions(+), 14 deletions(-) >> > >> >diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result >> >index 73fb03c..470ce5a 100644 >> >--- a/test/sql/func-recreate.result >> >+++ b/test/sql/func-recreate.result >> >@@ -12,31 +12,34 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') >> > fiber = require('fiber') >> > --- >> > ... >> >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) >> >+flag = true >> > --- >> > ... >> >-ch = fiber.channel(1) >> >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) >> > --- >> > ... >> >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) >> >+ch = fiber.channel(1) >> > --- >> > ... >> >-fiber.sleep(0.1) >> >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) >> > --- >> > ... >> >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) >> >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) >> > --- >> > - error: 'Failed to create function ''WAITFOR'': unable to create function due to >> > active statements' >> > ... >> >+flag = false >> >+--- >> >+... >> > ch:get() >> > --- >> > - metadata: >> >- - name: WAITFOR(0.2) >> >+ - name: WAITFOR() >> > type: integer >> > rows: >> >- - [0.2] >> >+ - [0] >> > ... >> >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) >> >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) >> > --- >> > ... >> >diff --git a/test/sql/func-recreate.test.lua b/test/sql/func-recreate.test.lua >> >index 753e9ca..d482fa9 100644 >> >--- a/test/sql/func-recreate.test.lua >> >+++ b/test/sql/func-recreate.test.lua >> >@@ -4,14 +4,15 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') >> > >> > -- Check errors during function create process >> > fiber = require('fiber') >> >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) >> >+flag = true >> >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) >> > >> > ch = fiber.channel(1) >> > >> >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) >> >-fiber.sleep(0.1) >> >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) >> > >> >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) >> >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) >> >+flag = false >> > ch:get() >> >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) >> >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) >> > >> >diff --git a/test/sql/suite.ini b/test/sql/suite.ini >> >index a8664c5..cc1d641 100644 >> >--- a/test/sql/suite.ini >> >+++ b/test/sql/suite.ini >> >@@ -11,4 +11,3 @@ release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua >> > disabled = sql-statN-index-drop.test.lua >> > pretest_clean = True >> > fragile = dll.test.lua ; gh-4427 >> >- func-recreate.test.lua ; gh-4384 >> >-- >> >2.7.4 >> > >> >> >> -- >> Alexander Tikhonov >> -- Alexander Tikhonov [-- Attachment #2: Type: text/html, Size: 6528 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-16 19:18 ` Nikita Pettik 2020-04-16 19:33 ` Alexander Tikhonov @ 2020-04-16 20:24 ` Mergen Imeev 2020-04-16 21:02 ` Nikita Pettik 1 sibling, 1 reply; 14+ messages in thread From: Mergen Imeev @ 2020-04-16 20:24 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches Hi! Thank you for review. My answer below. On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote: > On 16 Apr 22:09, Alexander Tikhonov wrote: > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. > > > > Did you verify that modified test still reproduces initial problem? > I hope so, but ask just in case. According to history, the main goal of this part of the test is to show that sql_create_function() throws proper error in case a new function is created when transaction is active. Since the error still here, we may say that the test works. > > > >Четверг, 16 апреля 2020, 22:07 +03:00 от imeevma@tarantool.org: > > > > > >Closes #4384 > > >--- > > >https://github.com/tarantool/tarantool/issues/4384 > > >https://github.com/tarantool/tarantool/tree/imeevma/gh-4384-fix-test-func-recreate > > > > > >@ChangeLog > > > - Fixed flaky test sql/func-recreate.test.lua (gh-4384). > > > > > > test/sql/func-recreate.result | 19 +++++++++++-------- > > > test/sql/func-recreate.test.lua | 11 ++++++----- > > > test/sql/suite.ini | 1 - > > > 3 files changed, 17 insertions(+), 14 deletions(-) > > > > > >diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result > > >index 73fb03c..470ce5a 100644 > > >--- a/test/sql/func-recreate.result > > >+++ b/test/sql/func-recreate.result > > >@@ -12,31 +12,34 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') > > > fiber = require('fiber') > > > --- > > > ... > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) > > >+flag = true > > > --- > > > ... > > >-ch = fiber.channel(1) > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > --- > > > ... > > >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) > > >+ch = fiber.channel(1) > > > --- > > > ... > > >-fiber.sleep(0.1) > > >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) > > > --- > > > ... > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > --- > > > - error: 'Failed to create function ''WAITFOR'': unable to create function due to > > > active statements' > > > ... > > >+flag = false > > >+--- > > >+... > > > ch:get() > > > --- > > > - metadata: > > >- - name: WAITFOR(0.2) > > >+ - name: WAITFOR() > > > type: integer > > > rows: > > >- - [0.2] > > >+ - [0] > > > ... > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > --- > > > ... > > >diff --git a/test/sql/func-recreate.test.lua b/test/sql/func-recreate.test.lua > > >index 753e9ca..d482fa9 100644 > > >--- a/test/sql/func-recreate.test.lua > > >+++ b/test/sql/func-recreate.test.lua > > >@@ -4,14 +4,15 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') > > > > > > -- Check errors during function create process > > > fiber = require('fiber') > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) > > >+flag = true > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > > > ch = fiber.channel(1) > > > > > >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) > > >-fiber.sleep(0.1) > > >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) > > > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > >+flag = false > > > ch:get() > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > > >diff --git a/test/sql/suite.ini b/test/sql/suite.ini > > >index a8664c5..cc1d641 100644 > > >--- a/test/sql/suite.ini > > >+++ b/test/sql/suite.ini > > >@@ -11,4 +11,3 @@ release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua > > > disabled = sql-statN-index-drop.test.lua > > > pretest_clean = True > > > fragile = dll.test.lua ; gh-4427 > > >- func-recreate.test.lua ; gh-4384 > > >-- > > >2.7.4 > > > > > > > > > -- > > Alexander Tikhonov > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-16 20:24 ` Mergen Imeev @ 2020-04-16 21:02 ` Nikita Pettik 2020-04-16 21:15 ` Mergen Imeev 0 siblings, 1 reply; 14+ messages in thread From: Nikita Pettik @ 2020-04-16 21:02 UTC (permalink / raw) To: Mergen Imeev; +Cc: tarantool-patches On 16 Apr 23:24, Mergen Imeev wrote: > Hi! Thank you for review. My answer below. > > On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote: > > On 16 Apr 22:09, Alexander Tikhonov wrote: > > > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. > > > > > > > Did you verify that modified test still reproduces initial problem? > > I hope so, but ask just in case. > According to history, the main goal of this part of the > test is to show that sql_create_function() throws proper > error in case a new function is created when transaction > is active. Since the error still here, we may say that > the test works. So the answer to my question is - no, you didn't test it. Better safe than sorry - I'd better verify that updated test still covers the initial problem. Thanks. > > > >Четверг, 16 апреля 2020, 22:07 +03:00 от imeevma@tarantool.org: > > > > > > > >Closes #4384 > > > >--- > > > >https://github.com/tarantool/tarantool/issues/4384 > > > >https://github.com/tarantool/tarantool/tree/imeevma/gh-4384-fix-test-func-recreate > > > > > > > >@ChangeLog > > > > - Fixed flaky test sql/func-recreate.test.lua (gh-4384). > > > > > > > > test/sql/func-recreate.result | 19 +++++++++++-------- > > > > test/sql/func-recreate.test.lua | 11 ++++++----- > > > > test/sql/suite.ini | 1 - > > > > 3 files changed, 17 insertions(+), 14 deletions(-) > > > > > > > >diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result > > > >index 73fb03c..470ce5a 100644 > > > >--- a/test/sql/func-recreate.result > > > >+++ b/test/sql/func-recreate.result > > > >@@ -12,31 +12,34 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') > > > > fiber = require('fiber') > > > > --- > > > > ... > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) > > > >+flag = true > > > > --- > > > > ... > > > >-ch = fiber.channel(1) > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > --- > > > > ... > > > >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) > > > >+ch = fiber.channel(1) > > > > --- > > > > ... > > > >-fiber.sleep(0.1) > > > >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) > > > > --- > > > > ... > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > --- > > > > - error: 'Failed to create function ''WAITFOR'': unable to create function due to > > > > active statements' > > > > ... > > > >+flag = false > > > >+--- > > > >+... > > > > ch:get() > > > > --- > > > > - metadata: > > > >- - name: WAITFOR(0.2) > > > >+ - name: WAITFOR() > > > > type: integer > > > > rows: > > > >- - [0.2] > > > >+ - [0] > > > > ... > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > --- > > > > ... > > > >diff --git a/test/sql/func-recreate.test.lua b/test/sql/func-recreate.test.lua > > > >index 753e9ca..d482fa9 100644 > > > >--- a/test/sql/func-recreate.test.lua > > > >+++ b/test/sql/func-recreate.test.lua > > > >@@ -4,14 +4,15 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') > > > > > > > > -- Check errors during function create process > > > > fiber = require('fiber') > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) > > > >+flag = true > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > > > > > ch = fiber.channel(1) > > > > > > > >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) > > > >-fiber.sleep(0.1) > > > >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) > > > > > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > >+flag = false > > > > ch:get() > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > > > > >diff --git a/test/sql/suite.ini b/test/sql/suite.ini > > > >index a8664c5..cc1d641 100644 > > > >--- a/test/sql/suite.ini > > > >+++ b/test/sql/suite.ini > > > >@@ -11,4 +11,3 @@ release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua > > > > disabled = sql-statN-index-drop.test.lua > > > > pretest_clean = True > > > > fragile = dll.test.lua ; gh-4427 > > > >- func-recreate.test.lua ; gh-4384 > > > >-- > > > >2.7.4 > > > > > > > > > > > > > -- > > > Alexander Tikhonov > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-16 21:02 ` Nikita Pettik @ 2020-04-16 21:15 ` Mergen Imeev 2020-04-16 22:04 ` Nikita Pettik 0 siblings, 1 reply; 14+ messages in thread From: Mergen Imeev @ 2020-04-16 21:15 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches On Thu, Apr 16, 2020 at 09:02:05PM +0000, Nikita Pettik wrote: > On 16 Apr 23:24, Mergen Imeev wrote: > > Hi! Thank you for review. My answer below. > > > > On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote: > > > On 16 Apr 22:09, Alexander Tikhonov wrote: > > > > > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. > > > > > > > > > > Did you verify that modified test still reproduces initial problem? > > > I hope so, but ask just in case. > > According to history, the main goal of this part of the > > test is to show that sql_create_function() throws proper > > error in case a new function is created when transaction > > is active. Since the error still here, we may say that > > the test works. > > So the answer to my question is - no, you didn't test it. > Better safe than sorry - I'd better verify that updated > test still covers the initial problem. Thanks. > Doesn't my answer says that we checked it? However, I agree that it is never wrong to check it again. > > > > >Четверг, 16 апреля 2020, 22:07 +03:00 от imeevma@tarantool.org: > > > > > > > > > >Closes #4384 > > > > >--- > > > > >https://github.com/tarantool/tarantool/issues/4384 > > > > >https://github.com/tarantool/tarantool/tree/imeevma/gh-4384-fix-test-func-recreate > > > > > > > > > >@ChangeLog > > > > > - Fixed flaky test sql/func-recreate.test.lua (gh-4384). > > > > > > > > > > test/sql/func-recreate.result | 19 +++++++++++-------- > > > > > test/sql/func-recreate.test.lua | 11 ++++++----- > > > > > test/sql/suite.ini | 1 - > > > > > 3 files changed, 17 insertions(+), 14 deletions(-) > > > > > > > > > >diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result > > > > >index 73fb03c..470ce5a 100644 > > > > >--- a/test/sql/func-recreate.result > > > > >+++ b/test/sql/func-recreate.result > > > > >@@ -12,31 +12,34 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') > > > > > fiber = require('fiber') > > > > > --- > > > > > ... > > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) > > > > >+flag = true > > > > > --- > > > > > ... > > > > >-ch = fiber.channel(1) > > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > > --- > > > > > ... > > > > >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) > > > > >+ch = fiber.channel(1) > > > > > --- > > > > > ... > > > > >-fiber.sleep(0.1) > > > > >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) > > > > > --- > > > > > ... > > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > > --- > > > > > - error: 'Failed to create function ''WAITFOR'': unable to create function due to > > > > > active statements' > > > > > ... > > > > >+flag = false > > > > >+--- > > > > >+... > > > > > ch:get() > > > > > --- > > > > > - metadata: > > > > >- - name: WAITFOR(0.2) > > > > >+ - name: WAITFOR() > > > > > type: integer > > > > > rows: > > > > >- - [0.2] > > > > >+ - [0] > > > > > ... > > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > > --- > > > > > ... > > > > >diff --git a/test/sql/func-recreate.test.lua b/test/sql/func-recreate.test.lua > > > > >index 753e9ca..d482fa9 100644 > > > > >--- a/test/sql/func-recreate.test.lua > > > > >+++ b/test/sql/func-recreate.test.lua > > > > >@@ -4,14 +4,15 @@ box.execute('pragma sql_default_engine=\''..engine..'\'') > > > > > > > > > > -- Check errors during function create process > > > > > fiber = require('fiber') > > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) > > > > >+flag = true > > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > > > > > > > ch = fiber.channel(1) > > > > > > > > > >-_ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) > > > > >-fiber.sleep(0.1) > > > > >+_ = fiber.create(function () ch:put(box.execute('select WAITFOR()')) end) > > > > > > > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > >+flag = false > > > > > ch:get() > > > > >-box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) > > > > >+box.internal.sql_create_function('WAITFOR', 'INT', function () while flag do fiber.sleep(0.01) end return 0 end) > > > > > > > > > >diff --git a/test/sql/suite.ini b/test/sql/suite.ini > > > > >index a8664c5..cc1d641 100644 > > > > >--- a/test/sql/suite.ini > > > > >+++ b/test/sql/suite.ini > > > > >@@ -11,4 +11,3 @@ release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua > > > > > disabled = sql-statN-index-drop.test.lua > > > > > pretest_clean = True > > > > > fragile = dll.test.lua ; gh-4427 > > > > >- func-recreate.test.lua ; gh-4384 > > > > >-- > > > > >2.7.4 > > > > > > > > > > > > > > > > > -- > > > > Alexander Tikhonov > > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-16 21:15 ` Mergen Imeev @ 2020-04-16 22:04 ` Nikita Pettik 2020-04-17 6:05 ` Alexander Tikhonov 2020-04-17 6:25 ` Mergen Imeev 0 siblings, 2 replies; 14+ messages in thread From: Nikita Pettik @ 2020-04-16 22:04 UTC (permalink / raw) To: Mergen Imeev; +Cc: tarantool-patches On 17 Apr 00:15, Mergen Imeev wrote: > On Thu, Apr 16, 2020 at 09:02:05PM +0000, Nikita Pettik wrote: > > On 16 Apr 23:24, Mergen Imeev wrote: > > > Hi! Thank you for review. My answer below. > > > > > > On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote: > > > > On 16 Apr 22:09, Alexander Tikhonov wrote: > > > > > > > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. > > > > > > > > > > > > > Did you verify that modified test still reproduces initial problem? > > > > I hope so, but ask just in case. > > > According to history, the main goal of this part of the > > > test is to show that sql_create_function() throws proper > > > error in case a new function is created when transaction > > > is active. Since the error still here, we may say that > > > the test works. > > > > So the answer to my question is - no, you didn't test it. > > Better safe than sorry - I'd better verify that updated > > test still covers the initial problem. Thanks. > > > Doesn't my answer says that we checked it? No, it doesn't. 'reproduce' means that you are supposed to checkout to branch, revert particular fix, run updated test without that fix and verify that test fails without fix. Any other actions do not refer to my original question. >However, I agree that it is never wrong to check it again. Please, check it for current fix and for any other test upgrade in future. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-16 22:04 ` Nikita Pettik @ 2020-04-17 6:05 ` Alexander Tikhonov 2020-04-17 12:45 ` Nikita Pettik 2020-04-17 6:25 ` Mergen Imeev 1 sibling, 1 reply; 14+ messages in thread From: Alexander Tikhonov @ 2020-04-17 6:05 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches [-- Attachment #1: Type: text/plain, Size: 2177 bytes --] >Пятница, 17 апреля 2020, 1:04 +03:00 от Nikita Pettik <korablev@tarantool.org>: > >On 17 Apr 00:15, Mergen Imeev wrote: >> On Thu, Apr 16, 2020 at 09:02:05PM +0000, Nikita Pettik wrote: >> > On 16 Apr 23:24, Mergen Imeev wrote: >> > > Hi! Thank you for review. My answer below. >> > > >> > > On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote: >> > > > On 16 Apr 22:09, Alexander Tikhonov wrote: >> > > > > >> > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. >> > > > > >> > > > >> > > > Did you verify that modified test still reproduces initial problem? >> > > > I hope so, but ask just in case. >> > > According to history, the main goal of this part of the >> > > test is to show that sql_create_function() throws proper >> > > error in case a new function is created when transaction >> > > is active. Since the error still here, we may say that >> > > the test works. >> > >> > So the answer to my question is - no, you didn't test it. >> > Better safe than sorry - I'd better verify that updated >> > test still covers the initial problem. Thanks. >> > >> Doesn't my answer says that we checked it? > >No, it doesn't. 'reproduce' means that you are supposed to checkout >to branch, revert particular fix, run updated test without that fix >and verify that test fails without fix. Any other actions do not refer >to my original question. I still don’t understand this check way. Please clarify: * checkout the branch with fix based on 2.2 release branch (understood) * revert particular fix (understood, but it is the same as checkout native 2.2 release branch, anyway ok) * «run updated test without that fix» — what does it mean «updated», ok if you mean previous revert to native 2.2 release branch, than we did it manually and in regular testings, check gitlab-ci for results, in both variants test fails Please point what is the difference between the way I understood and the way you suggest. > >>However, I agree that it is never wrong to check it again. > >Please, check it for current fix and for any other test >upgrade in future. > -- Alexander Tikhonov [-- Attachment #2: Type: text/html, Size: 3122 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-17 6:05 ` Alexander Tikhonov @ 2020-04-17 12:45 ` Nikita Pettik 2020-04-17 14:18 ` Mergen Imeev 0 siblings, 1 reply; 14+ messages in thread From: Nikita Pettik @ 2020-04-17 12:45 UTC (permalink / raw) To: Alexander Tikhonov; +Cc: tarantool-patches On 17 Apr 09:05, Alexander Tikhonov wrote: > > > > > >Пятница, 17 апреля 2020, 1:04 +03:00 от Nikita Pettik <korablev@tarantool.org>: > > > >On 17 Apr 00:15, Mergen Imeev wrote: > >> On Thu, Apr 16, 2020 at 09:02:05PM +0000, Nikita Pettik wrote: > >> > On 16 Apr 23:24, Mergen Imeev wrote: > >> > > Hi! Thank you for review. My answer below. > >> > > > >> > > On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote: > >> > > > On 16 Apr 22:09, Alexander Tikhonov wrote: > >> > > > > > >> > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. > >> > > > > > >> > > > > >> > > > Did you verify that modified test still reproduces initial problem? > >> > > > I hope so, but ask just in case. > >> > > According to history, the main goal of this part of the > >> > > test is to show that sql_create_function() throws proper > >> > > error in case a new function is created when transaction > >> > > is active. Since the error still here, we may say that > >> > > the test works. > >> > > >> > So the answer to my question is - no, you didn't test it. > >> > Better safe than sorry - I'd better verify that updated > >> > test still covers the initial problem. Thanks. > >> > > >> Doesn't my answer says that we checked it? > > > >No, it doesn't. 'reproduce' means that you are supposed to checkout > >to branch, revert particular fix, run updated test without that fix > >and verify that test fails without fix. Any other actions do not refer > >to my original question. > I still don’t understand this check way. Please clarify: > * checkout the branch with fix based on 2.2 release branch (understood) > * revert particular fix (understood, but it is the same as checkout native 2.2 release branch, anyway ok) False. You have to revert this fix: 14ba68f93 > * «run updated test without that fix» — what does it mean «updated», ok if you mean previous revert to native 2.2 release branch, than we did it manually and in regular testings, check gitlab-ci for results, in both variants test fails > Please point what is the difference between the way I understood and the way you suggest. > > > >>However, I agree that it is never wrong to check it again. > > > >Please, check it for current fix and for any other test > >upgrade in future. > > > > > -- > Alexander Tikhonov > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-17 12:45 ` Nikita Pettik @ 2020-04-17 14:18 ` Mergen Imeev 2020-04-17 15:50 ` Nikita Pettik 0 siblings, 1 reply; 14+ messages in thread From: Mergen Imeev @ 2020-04-17 14:18 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches Hi! I tested our solution. I run these two versions of the test on two different commits. The result showed that both versions work as intended. The main idea behind these tests is to show an error to user when sql_create_function() fails. To show that sql_create_function() fails I added one more call of the function. When we receive result of this call we may say which version of the function was executed. I didn't include check which shows that when function with the same name is created, it replaces old function. I checked it and I hope it is not needed to be included here. Old version: fiber = require('fiber') -- First version of the function box.internal.sql_create_function('WAITFOR', 'INT', function (n) fiber.sleep(n) return n end) ch = fiber.channel(1) _ = fiber.create(function () ch:put(box.sql.execute('select WAITFOR(0.2)')) end) fiber.sleep(0.1) -- Second version of the function box.internal.sql_create_function('WAITFOR', 'INT', function (n) return 3 end) ch:get() box.sql.execute('select WAITFOR(0.01)') New version: fiber = require('fiber') flag = true -- First version of the function box.internal.sql_create_function('WAITFOR', 'INT', function (n) while flag do fiber.sleep(0.01) end return n end) ch = fiber.channel(1) _ = fiber.create(function () ch:put(box.sql.execute('select WAITFOR(0.2)')) end) -- Second version of the function box.internal.sql_create_function('WAITFOR', 'INT', function (n) return 3 end) flag = false ch:get() box.sql.execute('select WAITFOR(0.01)') 1) Commit cfa7da0 (which is just before 14ba68f). Result of the old version: tarantool> box.internal.sql_create_function('WAITFOR', 'INT', function (n) return n + 1 end) --- ... tarantool> ch:get() --- - - [0.2] ... tarantool> box.sql.execute('select WAITFOR(0.01)') --- - - [0.01] ... Result of the new version: tarantool> box.internal.sql_create_function('WAITFOR', 'INT', function (n) return 3 end) --- ... tarantool> flag = false --- ... tarantool> ch:get() --- - - [0.2] ... tarantool> box.sql.execute('select WAITFOR(0.01)') --- - - [0.01] ... We can see, that in both cases sql_create_function() fails (since we receive 0.01 in the last coll in both cases), but we have no idea of this before we executed "select WAITFOR(0.01)". 2) Commit 14ba68f: Result of the old version: tarantool> box.internal.sql_create_function('WAITFOR', 'INT', function (n) return n + 1 end) --- - error: database is locked ... tarantool> ch:get() --- - - [0.2] ... tarantool> box.sql.execute('select WAITFOR(0.01)') --- - - [0.01] ... Result of the new version: tarantool> box.internal.sql_create_function('WAITFOR', 'INT', function (n) return 3 end) --- - error: database is locked ... tarantool> flag = false --- ... tarantool> ch:get() --- - - [0.2] ... tarantool> box.sql.execute('select WAITFOR(0.01)') --- - - [0.01] ... As we can see, an error appeared in both versions. I think this says that our version of the test works properly. On Fri, Apr 17, 2020 at 12:45:08PM +0000, Nikita Pettik wrote: > On 17 Apr 09:05, Alexander Tikhonov wrote: > > > > > > > > > > >Пятница, 17 апреля 2020, 1:04 +03:00 от Nikita Pettik <korablev@tarantool.org>: > > > > > >On 17 Apr 00:15, Mergen Imeev wrote: > > >> On Thu, Apr 16, 2020 at 09:02:05PM +0000, Nikita Pettik wrote: > > >> > On 16 Apr 23:24, Mergen Imeev wrote: > > >> > > Hi! Thank you for review. My answer below. > > >> > > > > >> > > On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote: > > >> > > > On 16 Apr 22:09, Alexander Tikhonov wrote: > > >> > > > > > > >> > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. > > >> > > > > > > >> > > > > > >> > > > Did you verify that modified test still reproduces initial problem? > > >> > > > I hope so, but ask just in case. > > >> > > According to history, the main goal of this part of the > > >> > > test is to show that sql_create_function() throws proper > > >> > > error in case a new function is created when transaction > > >> > > is active. Since the error still here, we may say that > > >> > > the test works. > > >> > > > >> > So the answer to my question is - no, you didn't test it. > > >> > Better safe than sorry - I'd better verify that updated > > >> > test still covers the initial problem. Thanks. > > >> > > > >> Doesn't my answer says that we checked it? > > > > > >No, it doesn't. 'reproduce' means that you are supposed to checkout > > >to branch, revert particular fix, run updated test without that fix > > >and verify that test fails without fix. Any other actions do not refer > > >to my original question. > > I still don’t understand this check way. Please clarify: > > * checkout the branch with fix based on 2.2 release branch (understood) > > * revert particular fix (understood, but it is the same as checkout native 2.2 release branch, anyway ok) > > False. You have to revert this fix: 14ba68f93 > > > * «run updated test without that fix» — what does it mean «updated», ok if you mean previous revert to native 2.2 release branch, than we did it manually and in regular testings, check gitlab-ci for results, in both variants test fails > > Please point what is the difference between the way I understood and the way you suggest. > > > > > >>However, I agree that it is never wrong to check it again. > > > > > >Please, check it for current fix and for any other test > > >upgrade in future. > > > > > > > > > -- > > Alexander Tikhonov > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-17 14:18 ` Mergen Imeev @ 2020-04-17 15:50 ` Nikita Pettik 0 siblings, 0 replies; 14+ messages in thread From: Nikita Pettik @ 2020-04-17 15:50 UTC (permalink / raw) To: Mergen Imeev; +Cc: tarantool-patches On 17 Apr 17:18, Mergen Imeev wrote: > Hi! I tested our solution. > > I run these two versions of the test on two different commits. > The result showed that both versions work as intended. Ok, thanks, it is what I asked for. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-16 22:04 ` Nikita Pettik 2020-04-17 6:05 ` Alexander Tikhonov @ 2020-04-17 6:25 ` Mergen Imeev 2020-04-17 6:31 ` Alexander Tikhonov 1 sibling, 1 reply; 14+ messages in thread From: Mergen Imeev @ 2020-04-17 6:25 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches On Thu, Apr 16, 2020 at 10:04:16PM +0000, Nikita Pettik wrote: > On 17 Apr 00:15, Mergen Imeev wrote: > > On Thu, Apr 16, 2020 at 09:02:05PM +0000, Nikita Pettik wrote: > > > On 16 Apr 23:24, Mergen Imeev wrote: > > > > Hi! Thank you for review. My answer below. > > > > > > > > On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote: > > > > > On 16 Apr 22:09, Alexander Tikhonov wrote: > > > > > > > > > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. > > > > > > > > > > > > > > > > Did you verify that modified test still reproduces initial problem? > > > > > I hope so, but ask just in case. > > > > According to history, the main goal of this part of the > > > > test is to show that sql_create_function() throws proper > > > > error in case a new function is created when transaction > > > > is active. Since the error still here, we may say that > > > > the test works. > > > > > > So the answer to my question is - no, you didn't test it. > > > Better safe than sorry - I'd better verify that updated > > > test still covers the initial problem. Thanks. > > > > > Doesn't my answer says that we checked it? > > No, it doesn't. 'reproduce' means that you are supposed to checkout > to branch, revert particular fix, run updated test without that fix > and verify that test fails without fix. Any other actions do not refer > to my original question. > > >However, I agree that it is never wrong to check it again. > > Please, check it for current fix and for any other test > upgrade in future. > To check this I changed the value in fiber.sleep() function on line 12 to 0.3 in the test before the patch. After that the test fails with this result: [001] Test failed! Result content mismatch: [001] --- sql/func-recreate.result Fri Apr 17 09:13:48 2020 [001] +++ sql/func-recreate.reject Fri Apr 17 09:17:52 2020 [001] @@ -21,13 +21,11 @@ [001] _ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) [001] --- [001] ... [001] -fiber.sleep(0.1) [001] +fiber.sleep(0.3) [001] --- [001] ... [001] box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) [001] --- [001] -- error: 'Failed to create function ''WAITFOR'': unable to create function due to [001] - active statements' [001] ... [001] ch:get() [001] --- As you can see, the error dissappeared. This time we got this fail after we changed the test, but it is possible to get it even without changing. This happens when the computher which runs this test is too heavy loaded. But this won't happen after the patch. Is this good enough? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua 2020-04-17 6:25 ` Mergen Imeev @ 2020-04-17 6:31 ` Alexander Tikhonov 0 siblings, 0 replies; 14+ messages in thread From: Alexander Tikhonov @ 2020-04-17 6:31 UTC (permalink / raw) To: Mergen Imeev; +Cc: tarantool-patches [-- Attachment #1: Type: text/plain, Size: 2934 bytes --] >Пятница, 17 апреля 2020, 9:25 +03:00 от Mergen Imeev <imeevma@tarantool.org>: > >On Thu, Apr 16, 2020 at 10:04:16PM +0000, Nikita Pettik wrote: >> On 17 Apr 00:15, Mergen Imeev wrote: >> > On Thu, Apr 16, 2020 at 09:02:05PM +0000, Nikita Pettik wrote: >> > > On 16 Apr 23:24, Mergen Imeev wrote: >> > > > Hi! Thank you for review. My answer below. >> > > > >> > > > On Thu, Apr 16, 2020 at 07:18:51PM +0000, Nikita Pettik wrote: >> > > > > On 16 Apr 22:09, Alexander Tikhonov wrote: >> > > > > > >> > > > > > Hi Mergen, thanks for the patch, I’ve checked it on 2.2 and it runs fine, patch LGTM. >> > > > > > >> > > > > >> > > > > Did you verify that modified test still reproduces initial problem? >> > > > > I hope so, but ask just in case. >> > > > According to history, the main goal of this part of the >> > > > test is to show that sql_create_function() throws proper >> > > > error in case a new function is created when transaction >> > > > is active. Since the error still here, we may say that >> > > > the test works. >> > > >> > > So the answer to my question is - no, you didn't test it. >> > > Better safe than sorry - I'd better verify that updated >> > > test still covers the initial problem. Thanks. >> > > >> > Doesn't my answer says that we checked it? >> >> No, it doesn't. 'reproduce' means that you are supposed to checkout >> to branch, revert particular fix, run updated test without that fix >> and verify that test fails without fix. Any other actions do not refer >> to my original question. >> >> >However, I agree that it is never wrong to check it again. >> >> Please, check it for current fix and for any other test >> upgrade in future. >> To check this I changed the value in fiber.sleep() function >on line 12 to 0.3 in the test before the patch. After that >the test fails with this result: > >[001] Test failed! Result content mismatch: >[001] --- sql/func-recreate.result Fri Apr 17 09:13:48 2020 >[001] +++ sql/func-recreate.reject Fri Apr 17 09:17:52 2020 >[001] @@ -21,13 +21,11 @@ >[001] _ = fiber.create(function () ch:put(box.execute('select WAITFOR(0.2)')) end) >[001] --- >[001] ... >[001] -fiber.sleep(0.1) >[001] +fiber.sleep(0.3) >[001] --- >[001] ... >[001] box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end) >[001] --- >[001] -- error: 'Failed to create function ''WAITFOR'': unable to create function due to >[001] - active statements' >[001] ... >[001] ch:get() >[001] --- > >As you can see, the error dissappeared. > >This time we got this fail after we changed the test, but >it is possible to get it even without changing. This >happens when the computher which runs this test is too >heavy loaded. But this won't happen after the patch. > >Is this good enough? > I can only agree here, I’ve tried it too before the patch was created, while the issue was researched. -- Alexander Tikhonov [-- Attachment #2: Type: text/html, Size: 3849 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-04-17 15:50 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-16 19:07 [Tarantool-patches] [PATCH v1 1/1] lua: fix test sql/func-recreate.test.lua imeevma 2020-04-16 19:09 ` Alexander Tikhonov 2020-04-16 19:18 ` Nikita Pettik 2020-04-16 19:33 ` Alexander Tikhonov 2020-04-16 20:24 ` Mergen Imeev 2020-04-16 21:02 ` Nikita Pettik 2020-04-16 21:15 ` Mergen Imeev 2020-04-16 22:04 ` Nikita Pettik 2020-04-17 6:05 ` Alexander Tikhonov 2020-04-17 12:45 ` Nikita Pettik 2020-04-17 14:18 ` Mergen Imeev 2020-04-17 15:50 ` Nikita Pettik 2020-04-17 6:25 ` Mergen Imeev 2020-04-17 6:31 ` Alexander Tikhonov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox