From ea534acf98e29f0b6350b260e276dc62505a08e0 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev <i.zolotarev@corp.mail.ru> Date: Wed, 19 May 2021 18:38:22 +0300 Subject: [PATCH 1/9] fiber.yield() call removed from quantile --- metrics/quantile.lua | 19 +++++++------------ test/quantile_test.lua | 27 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/metrics/quantile.lua b/metrics/quantile.lua index cce24e59..81eaafcc 100644 --- a/metrics/quantile.lua +++ b/metrics/quantile.lua @@ -1,11 +1,12 @@ -local fiber = require('fiber') local ffi = require('ffi') local quantile = {} -ffi.cdef[[ - typedef struct {int Delta, Width; double Value; } sample; -]] +if not pcall(ffi.typeof, "sample") then + ffi.cdef[[ + typedef struct sample {int Delta, Width; double Value; } sample; + ]] +end local sample_constructor = ffi.typeof('sample') @@ -134,8 +135,8 @@ function stream:merge(samples, len) local i = 1 local r = 0 for z = 1, len do - if i % 1000 == 0 then - fiber.yield() + if i % 1000 == 0 then + require'fiber'.yield() end local sample = samples[z-1] for j = i, s.l_len do @@ -165,9 +166,6 @@ function stream:query(q) local p = s.l[0] local r = 0 for i = 1, s.l_len do - if i % 500 == 0 then - fiber.yield() - end local c = s.l[i] if r + c.Width + c.Delta > t then return p.Value @@ -189,9 +187,6 @@ function stream:compress() local r = s.n - x.Width for i = s.l_len - 1, 1, -1 do - if i % 1000 == 0 then - fiber.yield() - end local c = make_sample(0) sample_copy(c, s.l[i]) if c.Width + x.Width + x.Delta <= s.f(s, r) then diff --git a/test/quantile_test.lua b/test/quantile_test.lua index 96cdef07..ca6c58ef 100644 --- a/test/quantile_test.lua +++ b/test/quantile_test.lua @@ -1,4 +1,5 @@ local quantile = require('metrics.quantile') +local fiber = require('fiber') local ffi = require('ffi') local t = require('luatest') local g = t.group('quantile') @@ -104,3 +105,29 @@ g.test_package_reload = function() local ok, quantile_package = pcall(require, 'metrics.quantile') t.assert(ok, quantile_package) end + + +g.test_fiber_yield = function() + local + q1 = quantile.NewTargeted({[0.5]=0.01, [0.9]=0.01, [0.99]=0.01}) + + for _=1,1e6 do + t.assert(q1.b_len < q1.__max_samples) + quantile.Insert(q1, math.random(1)) + end + + for _=1,500 do + fiber.create(function() + for _=1,1e2 do + t.assert(q1.b_len < q1.__max_samples) + quantile.Insert(q1, math.random(1000)) + end + end) + end + + for _=1,1e6 do + t.assert(q1.b_len < q1.__max_samples) + quantile.Insert(q1, math.random(1000)) + end + +end From 51e9e637225bb7779bbd47dea6e8142b2e8ac5f6 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev <i.zolotarev@corp.mail.ru> Date: Wed, 19 May 2021 19:28:43 +0300 Subject: [PATCH 2/9] FIxed test --- metrics/quantile.lua | 3 --- 1 file changed, 3 deletions(-) diff --git a/metrics/quantile.lua b/metrics/quantile.lua index 81eaafcc..5b79354b 100644 --- a/metrics/quantile.lua +++ b/metrics/quantile.lua @@ -135,9 +135,6 @@ function stream:merge(samples, len) local i = 1 local r = 0 for z = 1, len do - if i % 1000 == 0 then - require'fiber'.yield() - end local sample = samples[z-1] for j = i, s.l_len do local c = s.l[j] From e990843e33bd43cb9b809fd8c5efd9efae821d2a Mon Sep 17 00:00:00 2001 From: Igor Zolotarev <i.zolotarev@corp.mail.ru> Date: Thu, 20 May 2021 16:22:49 +0300 Subject: [PATCH 3/9] More compact failing test --- metrics/quantile.lua | 1 + test/quantile_test.lua | 9 ++------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/metrics/quantile.lua b/metrics/quantile.lua index 5b79354b..110cfb5c 100644 --- a/metrics/quantile.lua +++ b/metrics/quantile.lua @@ -135,6 +135,7 @@ function stream:merge(samples, len) local i = 1 local r = 0 for z = 1, len do + if i%500 ==0 then require'fiber'.yield() end local sample = samples[z-1] for j = i, s.l_len do local c = s.l[j] diff --git a/test/quantile_test.lua b/test/quantile_test.lua index ca6c58ef..52138f8c 100644 --- a/test/quantile_test.lua +++ b/test/quantile_test.lua @@ -111,12 +111,7 @@ g.test_fiber_yield = function() local q1 = quantile.NewTargeted({[0.5]=0.01, [0.9]=0.01, [0.99]=0.01}) - for _=1,1e6 do - t.assert(q1.b_len < q1.__max_samples) - quantile.Insert(q1, math.random(1)) - end - - for _=1,500 do + for _=1,5 do fiber.create(function() for _=1,1e2 do t.assert(q1.b_len < q1.__max_samples) @@ -125,7 +120,7 @@ g.test_fiber_yield = function() end) end - for _=1,1e6 do + for _=1,1e1 do t.assert(q1.b_len < q1.__max_samples) quantile.Insert(q1, math.random(1000)) end From 3bc30245a10b817fafbf80950b14e7d47fdc2d4f Mon Sep 17 00:00:00 2001 From: Igor Zolotarev <i.zolotarev@corp.mail.ru> Date: Thu, 20 May 2021 18:10:03 +0300 Subject: [PATCH 4/9] New stress test --- .github/workflows/deploy_to_packagecloud.yml | 11 +++++++++++ Makefile | 6 +++++- metrics/quantile.lua | 1 - test/quantile_test.lua | 14 ++++++++------ 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/.github/workflows/deploy_to_packagecloud.yml b/.github/workflows/deploy_to_packagecloud.yml index a6b2d654..67f7072b 100644 --- a/.github/workflows/deploy_to_packagecloud.yml +++ b/.github/workflows/deploy_to_packagecloud.yml @@ -5,7 +5,18 @@ on: - '*' jobs: + stress-test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: install Tarantool + run: | + curl -L https://tarantool.io/installer.sh | sudo VER=2.4 bash + sudo apt install -y tarantool-dev + - name: stress test + run: make stress_test deploy: + needs: stress-test strategy: fail-fast: false matrix: diff --git a/Makefile b/Makefile index a43b1e8d..64997c0c 100644 --- a/Makefile +++ b/Makefile @@ -24,11 +24,15 @@ test: .rocks .PHONY: test_with_coverage_report test_with_coverage_report: .rocks rm -f tmp/luacov.*.out* - .rocks/bin/luatest --coverage -v --shuffle group + .rocks/bin/luatest --coverage -v --shuffle group --exclude stress .rocks/bin/luacov . echo grep -A999 '^Summary' tmp/luacov.report.out +.PHONY: stress_test +stress_test: .rocks + .rocks/bin/luatest --pattern stress + .PHONY: test_promtool test_promtool: .rocks tarantool test/promtool_test.lua diff --git a/metrics/quantile.lua b/metrics/quantile.lua index 110cfb5c..5b79354b 100644 --- a/metrics/quantile.lua +++ b/metrics/quantile.lua @@ -135,7 +135,6 @@ function stream:merge(samples, len) local i = 1 local r = 0 for z = 1, len do - if i%500 ==0 then require'fiber'.yield() end local sample = samples[z-1] for j = i, s.l_len do local c = s.l[j] diff --git a/test/quantile_test.lua b/test/quantile_test.lua index 52138f8c..e840d897 100644 --- a/test/quantile_test.lua +++ b/test/quantile_test.lua @@ -106,12 +106,15 @@ g.test_package_reload = function() t.assert(ok, quantile_package) end +g.test_fiber_yield_stress = function() + local q1 = quantile.NewTargeted({[0.5]=0.01, [0.9]=0.01, [0.99]=0.01}) -g.test_fiber_yield = function() - local - q1 = quantile.NewTargeted({[0.5]=0.01, [0.9]=0.01, [0.99]=0.01}) + for _=1,1e6 do + t.assert(q1.b_len < q1.__max_samples) + quantile.Insert(q1, math.random(1)) + end - for _=1,5 do + for _=1,500 do fiber.create(function() for _=1,1e2 do t.assert(q1.b_len < q1.__max_samples) @@ -120,9 +123,8 @@ g.test_fiber_yield = function() end) end - for _=1,1e1 do + for _=1,1e6 do t.assert(q1.b_len < q1.__max_samples) quantile.Insert(q1, math.random(1000)) end - end From da16641886306ee2d87e25f17104259c737127d5 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev <i.zolotarev@corp.mail.ru> Date: Thu, 20 May 2021 18:18:08 +0300 Subject: [PATCH 5/9] Changelog updated --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04cc063f..9b8c375c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - cpu metrics hot reload [#228](https://github.com/tarantool/metrics/issues/228) - cartridge metrics role fails to start without http [#225](https://github.com/tarantool/metrics/issues/225) +- quantile overflow after `fiber.yield()` [#225](https://github.com/tarantool/metrics/issues/235) ## [0.8.0] - 2021-04-13 ### Added From b1eb61973e077b5c17888597e0c238ea0509c069 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev <i.zolotarev@corp.mail.ru> Date: Thu, 20 May 2021 18:44:45 +0300 Subject: [PATCH 6/9] Stress test moved to tests stage --- .github/workflows/deploy_to_packagecloud.yml | 11 ----------- .github/workflows/test.yml | 13 +++++++++++++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/.github/workflows/deploy_to_packagecloud.yml b/.github/workflows/deploy_to_packagecloud.yml index 67f7072b..a6b2d654 100644 --- a/.github/workflows/deploy_to_packagecloud.yml +++ b/.github/workflows/deploy_to_packagecloud.yml @@ -5,18 +5,7 @@ on: - '*' jobs: - stress-test: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - name: install Tarantool - run: | - curl -L https://tarantool.io/installer.sh | sudo VER=2.4 bash - sudo apt install -y tarantool-dev - - name: stress test - run: make stress_test deploy: - needs: stress-test strategy: fail-fast: false matrix: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 926a3b81..a5535b4a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,6 +3,8 @@ on: push: branches: - '*' + tags: + - '*' jobs: test: strategy: @@ -65,3 +67,14 @@ jobs: run: | GO111MODULE=on go get github.com/prometheus/prometheus/cmd/promtool@a6be548dbc17780d562a39c0e4bd0bd4c00ad6e2 make test_promtool + stress-test: + runs-on: ubuntu-latest + if: startsWith(github.ref, 'refs/tags') + steps: + - uses: actions/checkout@v2 + - name: install Tarantool + run: | + curl -L https://tarantool.io/installer.sh | sudo VER=2.4 bash + sudo apt install -y tarantool-dev + - name: stress test + run: make stress_test From a0028bbd7d232bd8a162e37c08c4e15d4527be62 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev <i.zolotarev@corp.mail.ru> Date: Mon, 24 May 2021 14:24:53 +0300 Subject: [PATCH 7/9] More compact valid test --- .github/workflows/test.yml | 14 +------------- Makefile | 4 ---- metrics/quantile.lua | 1 + test/quantile_test.lua | 15 +++++---------- 4 files changed, 7 insertions(+), 27 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a5535b4a..285ff3fa 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,8 +3,7 @@ on: push: branches: - '*' - tags: - - '*' + jobs: test: strategy: @@ -67,14 +66,3 @@ jobs: run: | GO111MODULE=on go get github.com/prometheus/prometheus/cmd/promtool@a6be548dbc17780d562a39c0e4bd0bd4c00ad6e2 make test_promtool - stress-test: - runs-on: ubuntu-latest - if: startsWith(github.ref, 'refs/tags') - steps: - - uses: actions/checkout@v2 - - name: install Tarantool - run: | - curl -L https://tarantool.io/installer.sh | sudo VER=2.4 bash - sudo apt install -y tarantool-dev - - name: stress test - run: make stress_test diff --git a/Makefile b/Makefile index 64997c0c..9577a6e5 100644 --- a/Makefile +++ b/Makefile @@ -29,10 +29,6 @@ test_with_coverage_report: .rocks echo grep -A999 '^Summary' tmp/luacov.report.out -.PHONY: stress_test -stress_test: .rocks - .rocks/bin/luatest --pattern stress - .PHONY: test_promtool test_promtool: .rocks tarantool test/promtool_test.lua diff --git a/metrics/quantile.lua b/metrics/quantile.lua index 5b79354b..4c71dd83 100644 --- a/metrics/quantile.lua +++ b/metrics/quantile.lua @@ -135,6 +135,7 @@ function stream:merge(samples, len) local i = 1 local r = 0 for z = 1, len do + if i % 1000 == 0 then require'fiber'.yield() end local sample = samples[z-1] for j = i, s.l_len do local c = s.l[j] diff --git a/test/quantile_test.lua b/test/quantile_test.lua index e840d897..04654d72 100644 --- a/test/quantile_test.lua +++ b/test/quantile_test.lua @@ -106,15 +106,10 @@ g.test_package_reload = function() t.assert(ok, quantile_package) end -g.test_fiber_yield_stress = function() - local q1 = quantile.NewTargeted({[0.5]=0.01, [0.9]=0.01, [0.99]=0.01}) +g.test_fiber_yield = function() + local q1 = quantile.NewTargeted({[0.5]=0.01, [0.9]=0.01, [0.99]=0.01}, 1000) - for _=1,1e6 do - t.assert(q1.b_len < q1.__max_samples) - quantile.Insert(q1, math.random(1)) - end - - for _=1,500 do + for _=1,10 do fiber.create(function() for _=1,1e2 do t.assert(q1.b_len < q1.__max_samples) @@ -123,8 +118,8 @@ g.test_fiber_yield_stress = function() end) end - for _=1,1e6 do + for _=1,10 do t.assert(q1.b_len < q1.__max_samples) - quantile.Insert(q1, math.random(1000)) + quantile.Insert(q1, math.random(1)) end end From 37b452c0741169bfb8f8d496340ea2d598089d47 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev <i.zolotarev@corp.mail.ru> Date: Mon, 24 May 2021 14:27:36 +0300 Subject: [PATCH 8/9] Remove some old code --- .github/workflows/test.yml | 1 - Makefile | 2 +- metrics/quantile.lua | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 285ff3fa..926a3b81 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,7 +3,6 @@ on: push: branches: - '*' - jobs: test: strategy: diff --git a/Makefile b/Makefile index 9577a6e5..a43b1e8d 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,7 @@ test: .rocks .PHONY: test_with_coverage_report test_with_coverage_report: .rocks rm -f tmp/luacov.*.out* - .rocks/bin/luatest --coverage -v --shuffle group --exclude stress + .rocks/bin/luatest --coverage -v --shuffle group .rocks/bin/luacov . echo grep -A999 '^Summary' tmp/luacov.report.out diff --git a/metrics/quantile.lua b/metrics/quantile.lua index 4c71dd83..5b79354b 100644 --- a/metrics/quantile.lua +++ b/metrics/quantile.lua @@ -135,7 +135,6 @@ function stream:merge(samples, len) local i = 1 local r = 0 for z = 1, len do - if i % 1000 == 0 then require'fiber'.yield() end local sample = samples[z-1] for j = i, s.l_len do local c = s.l[j] From df7e32f3c8e35052b7ad6d47480cd88b547cbfeb Mon Sep 17 00:00:00 2001 From: Igor Zolotarev <i.zolotarev@corp.mail.ru> Date: Mon, 24 May 2021 14:29:00 +0300 Subject: [PATCH 9/9] Fix CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b8c375c..ac0e9240 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - cpu metrics hot reload [#228](https://github.com/tarantool/metrics/issues/228) - cartridge metrics role fails to start without http [#225](https://github.com/tarantool/metrics/issues/225) -- quantile overflow after `fiber.yield()` [#225](https://github.com/tarantool/metrics/issues/235) +- quantile overflow after `fiber.yield()` [#235](https://github.com/tarantool/metrics/issues/235) ## [0.8.0] - 2021-04-13 ### Added