scylladb / seastar

High performance server-side application framework
http://seastar.io
Apache License 2.0
8.28k stars 1.54k forks source link

slab_test fails #105

Open npmadhu opened 8 years ago

npmadhu commented 8 years ago

slab_test fails - able to reproduce on 2 machines - trying to understand logic in the allocator- specifically on the following: core/slab.hh: 64 [slab_page_desc] 72 _free_objects.reserve(objects - 1); 73 for (auto i = 1u; i < objects; i++) {

objects is 1 and nothing is populated and kept in _free_objects.

slab[ 39] size: 1048576 per-slab-page: 1 slab_test: ./core/slab.hh:112: void\* slab_page_desc::allocate_object(): Assertion `!_free_objects.empty()' failed.
yurai007 commented 8 years ago

This test fails on my machine too. In test_allocation_1 we perform in loop 5 allocations by calling 5x slab.create(1MB).

First allocation leads to new slab_class creation from page (slab_class::create_from_new_page) which leads to creation new slab_page_desc with place for only 1 object (because in our test_allocation_1 we have max_object_size == request size passing by create in test == 1MB).

Second allocation leads to using previously allocated slab_class (because this slab_class is not empty) so slab_class::create is called and in consequence slab_page_desc::allocate_object (desc.allocate_object()). But this desc was used in previous iteration and there is no place for second object so _free_objects .empty() is true. Finally assert fails.

npmadhu commented 8 years ago

If I understand the test case and the code correctly, the object_size chosen is 1MB - the number of 1 MB objects per page is 1. The code does not populate the vector for the case of 1 object in the page and create does not allocate any objects that have not been pre-assigned in free_objects - in essence trying to allocate 5 objects when only 1 exists in the pool is bound to fail - it will only work for smaller objects such that adequate number of instances exist in the pool.

If we are not supposed to exceed the pool then the test case is not correct and allocator assert though inconvenient is fine.

Is there a feature missing or can someone suggest what they intended to do with the test case to begin with ?

npmadhu commented 8 years ago

Have to confirm and see if this test case passed when it was added and if it broke after the following reclaimer changes.

https://github.com/scylladb/seastar/commit/89ec1f8f6a82d113b417f7333497978553b840c2

yurai007 commented 8 years ago

Finally I managed to solve this issue. We must ommit pushing new slab_page_desc to _free_slab_pages if it has place for only 1 object. But it's not enaugh. After this simple change I belive some bug appears in ~slab_allocator(). We delete desc which is still on intrusive list _free_slab_pages in _slab_classes. I think it's forbidden for intrusive containers and in consequence we break assert in boost internals (Assertion `!hook.is_linked()' failed). It is fixed by clearing _slab_classes in ~slab_allocator(). Now slab_tests passes on my machine (Arch linux, gcc 5.3, boost 1.60, kernel 4.3). I attached diff as *.txt.

current.txt

Seastar devs - please review those changes and check if issue is reproducible on different SW version. Attached @raphaelsc

npmadhu commented 8 years ago

Thanks - test case works fine - I don't understand the following well enough -

if (objects > 1) _free_slab_pages.push_front(*desc);

npmadhu commented 8 years ago

Did you enable the other test cases and do they pass?

Can't get all of them to pass as one - issues with clean up.

npmadhu commented 8 years ago

Got it to work - sorry it was something that was a local change - all good!

Thanks!