Result of new ModState ignored

ThomasArts commented 5 years ago

This looks like a harmless unused variable, but ModState0 may indicate a new workerpool added that is "forgotten" about in the system.

As a consequence, in some settings, queue_work may fail unexpectedly

https://github.com/basho/riak_core/blob/22b92dc14dc47a7f7f57bf642a45adc791bb9234/src/riak_core_vnode.erl#L233

martinsumner commented 5 years ago

Currently looking to create some new riak_test tests for develop-2.9 to properly highlight this and some related issues:

behaviour with folds and different pool strategy
rebuild in parallel mode, and the need for fold_objects to read binary objects
handling an aae_fold query when tictac aae is disabled on a node

Previously it was assumed that verify_tictac_aae was testing this functionality - but it clearly isn't.

martinsumner commented 5 years ago

@ThomasArts Something has gone wrong with the merge here as in develop-2.9 ModState0 is used correctly, being used in L248.

https://github.com/basho/riak_core/commit/2d85e34d7dd342d104c9d056e33b061e26ce6034

I wrote a test for develop-2.9 assuming it was broken, to try and expose it was broken - and the test failed because it discovered a different bug.

That undiscovered bug is fixed, and I've proven the test would expose an issue if the ModState was not correctly set in the future. So that test will go into develop-2.9 and be merged into develop-3.0, and we have coverage of this scenario going forward.

basho / riak_core

Result of new ModState ignored #939