Select with bucket_id specified leads to map-reduce #220

sharonovd · 2021-10-13T10:06:53Z

See full repro (cartridge app, insert.lua, loader.lua) attached.

Long story short:
crud.select('space_1', {{'==', 'secondary', 's_test_1'}}, { bucket_id = vshard.router.bucket_id_mpcrc32('s_test_1' )})
leads to map-reduce. I can see it in box.stat of both shards, and it starts throwing errors if I monkeypatch _G._crud.select_on_storage on one of the shards.

crud 0.8.0

The text was updated successfully, but these errors were encountered:

sharonovd · 2021-10-13T10:10:06Z

crudapp.zip

sharonovd · 2021-10-13T10:19:40Z

I suspect pairs is affected as well - so it should be fixed alongside

sharonovd · 2021-10-13T11:35:13Z

See #221 fjr a quickfix suggestion

I don't want to lean on box.stat() information here, because I don't control all iproto calls to storages: say, vshard rebalancer may perform them in background. Instead, I wrapped particular storage function I'm interested in. The goal is to be able to determine how much storages is involved into a select/pairs request. Part of #220

The bug is simple: crud ignores provided bucket_id, when unable to determine it itself. For example, when no conditions are given or when given condition involves a secondary index, which is not entirely in the primary index. It leads to incorrect select/pairs result: tuples are collected from all replicasets, while should be collected from one storage pointed by bucket_id. Second, it involves all replicasets into the request processing (performs map-reduce) that may dramatically drop performance. One existing test case was changed: 'test_opts_not_damaged' in ipairs_test.lua. The crud.pairs() request in this test case was affected by the problem and incorrect result was expected. The idea of the fix is suggested by Michael Filonenko in PR #221. Fixes #220

I don't want to lean on box.stat() information here, because I don't control all iproto calls to storages: say, vshard rebalancer may perform them in background. Instead, I wrapped particular storage function I'm interested in. The goal is to be able to determine how much storages are involved into a select/pairs request. Part of #220

The bug is simple: crud ignores provided bucket_id, when unable to determine it itself. For example, when no conditions are given or when given condition involves a secondary index, which is not entirely in the primary index. It leads to incorrect select/pairs result: tuples are collected from all replicasets, while should be collected from one storage pointed by bucket_id. Second, it involves all replicasets into the request processing (performs map-reduce) that may dramatically drop performance. One existing test case was changed: 'test_opts_not_damaged' in ipairs_test.lua. The crud.pairs() request in this test case was affected by the problem and incorrect result was expected. The idea of the fix is suggested by Michael Filonenko in PR #221. Fixes #220

I don't want to lean on box.stat() information here, because I don't control all iproto calls to storages: say, vshard rebalancer may perform them in background. Instead, I wrapped particular storage function I'm interested in. The goal is to be able to determine how much storages are involved into a select/pairs request. It is implemented as a helper for testing, but hopefully we'll implement some nice statistics as part of the module in a future (see #224). Part of #220

The bug is simple: crud ignores provided bucket_id, when unable to determine it itself. For example, when no conditions are given or when given condition involves a secondary index, which is not entirely in the primary index. It leads to incorrect select/pairs result: tuples are collected from all replicasets, while should be collected from one replicaset pointed by bucket_id. Second, it involves all replicasets into the request processing (performs map-reduce) that may dramatically drop performance. One existing test case was changed: 'test_opts_not_damaged' in ipairs_test.lua. The crud.pairs() request in this test case was affected by the problem and incorrect result was expected. The idea of the fix is suggested by Michael Filonenko in PR #221. Nice suggestions were given by Sergey Bronnikov (see PR #222). Fixes #220

A merge source has the same API as merger itself, so there is no difference in hehaviour between a merger created from one source and this source itself. However there is no overhead for creating key_def, merger and passing tuples over the merger. This opmimization gives me 13% boost on the case from #220. Follows up #220

A merge source has the same API as merger itself, so there is no difference in behaviour between a merger created from one source and this source itself. However there is no overhead for creating key_def, merger and passing tuples over the merger. This optimization gives me 13% boost on the case from #220. Follows up #220

The bug is simple: crud ignores provided bucket_id, when unable to determine it itself. For example, when no conditions are given or when given condition involves a secondary index, which is not entirely in the primary index. It leads to incorrect select/pairs result: tuples are collected from all replicasets, while should be collected from one replicaset pointed by bucket_id. Second, it involves all replicasets into the request processing (performs map-reduce) that may dramatically drop performance. One existing test case was changed: 'test_opts_not_damaged' in ipairs_test.lua. The crud.pairs() request in this test case was affected by the problem and incorrect result was expected. The idea of the fix is suggested by Michael Filonenko in PR #221. Nice suggestions were given by Sergey Bronnikov (see PR #222). Fixes #220

I don't want to lean on box.stat() information here, because I don't control all iproto calls to storages: say, vshard rebalancer may perform them in background. Instead, I wrapped particular storage function I'm interested in. The goal is to be able to determine how much storages are involved into a select/pairs request. It is implemented as a helper for testing, but hopefully we'll implement some nice statistics as part of the module in a future (see #224). Part of #220

The bug is simple: crud ignores provided bucket_id, when unable to determine it itself. For example, when no conditions are given or when given condition involves a secondary index, which is not entirely in the primary index. It leads to incorrect select/pairs result: tuples are collected from all replicasets, while should be collected from one replicaset pointed by bucket_id. Second, it involves all replicasets into the request processing (performs map-reduce) that may dramatically drop performance. One existing test case was changed: 'test_opts_not_damaged' in ipairs_test.lua. The crud.pairs() request in this test case was affected by the problem and incorrect result was expected. The idea of the fix is suggested by Michael Filonenko in PR #221. Nice suggestions were given by Sergey Bronnikov (see PR #222). Fixes #220

A merge source has the same API as merger itself, so there is no difference in behaviour between a merger created from one source and this source itself. However there is no overhead for creating key_def, merger and passing tuples over the merger. This optimization gives me 13% boost on the case from #220. Follows up #220

This patch reworks existing performance tests and adds new cases: select for equal conditions for primary and secondary indexes (including #220 case). It also adds corresponding vshard test cases to compare performance. Comparison may be not exactly precise since vshard test functions use naive mergers, but should at least estimate basic differences. Closes #225

This patch adds new cases for performance tests: select for equal conditions for primary and secondary indexes (including known bucket_id case from #220), and adds corresponding vshard test cases to compare performance. Comparison may be not exactly precise since vshard test functions use naive mergers, but should at least estimate basic differences. Test run on HP ProBook 440 G7 i7/16Gb/256SSD shows that CRUD module is 4-5 times slower than vshard calls for prepared functions for select and 2 times slower for insert. Closes #225

This patch adds new cases for performance tests: select for equal conditions for primary and secondary indexes (including known bucket_id case from #220), and adds corresponding vshard test cases to compare performance. Comparison may be not exactly precise since vshard test functions use naive mergers, but should at least estimate basic differences. Test run on HP ProBook 440 G7 i7/16Gb/256SSD shows that CRUD module is 4-8 times slower than vshard calls for prepared functions for select and 2 times slower for insert. Closes #225

This patch adds new cases for performance tests: select for equal conditions for primary and secondary indexes (including known bucket_id case from #220), and adds corresponding vshard test cases to compare performance. Comparison may be not exactly precise since vshard test functions use naive mergers, but should at least estimate basic differences. Test run on HP ProBook 440 G7 i7/16Gb/256SSD shows that CRUD module is 3-6 times slower than vshard calls for prepared functions for select and 1.6 times slower for insert. Closes #225

This patch adds new cases for performance tests: select for equal conditions for primary and secondary indexes (including known bucket_id case from #220), and adds corresponding vshard test cases to compare performance. Comparison may be not exactly precise since approaches to vshard stored procedures may vary, but it should at least estimate basic differences. Test run on HP ProBook 440 G7 i7/16Gb/256SSD shows that CRUD module is 3-6 times slower than vshard calls for prepared functions for select and 1.6 times slower for insert. Closes #225

sharonovd added bug Something isn't working customer performance labels Oct 13, 2021

Totktonada self-assigned this Oct 13, 2021

Totktonada mentioned this issue Oct 14, 2021

select/pairs: don't ignore provided bucket_id #222

Merged

Totktonada added the teamE label Oct 14, 2021

Totktonada mentioned this issue Oct 19, 2021

merger: don't create key_def/merger for one source #226

Merged

Totktonada closed this as completed in #222 Oct 19, 2021

This was referenced Nov 11, 2021

Create a test with comparison vshard.call() and CRUD performance #225

Closed

Optimize select #234

Open

DifferentialOrange mentioned this issue Feb 25, 2022

Add performance tests to compare crud and vshard #251

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Select with bucket_id specified leads to map-reduce #220

Select with bucket_id specified leads to map-reduce #220

sharonovd commented Oct 13, 2021 •

edited

Loading

sharonovd commented Oct 13, 2021

sharonovd commented Oct 13, 2021

sharonovd commented Oct 13, 2021

Select with bucket_id specified leads to map-reduce #220

Select with bucket_id specified leads to map-reduce #220

Comments

sharonovd commented Oct 13, 2021 • edited Loading

sharonovd commented Oct 13, 2021

sharonovd commented Oct 13, 2021

sharonovd commented Oct 13, 2021

sharonovd commented Oct 13, 2021 •

edited

Loading