Debug Assertion failure in btree_pack_post_loop() -> mini_destroy_unused() reporting mismatch in (mini->num_extents == mini->num_batches) #458
Assignees
Labels
bug
Something isn't working
Comments
gapisback
added a commit
that referenced
this issue
Oct 13, 2022
gapisback
added a commit
that referenced
this issue
Oct 13, 2022
This commit adds a stress test to exercise large #s of inserts,
from a main connection to Splinter, or from multiple threads.
Some test-configuration is provided
o --num-inserts; default 20 Million
o --num-threads default 8, each thread inserts --num-insert KV-pairs
All combinations of seq/random keys/value-pairs are supported.
Currently, no new bugs were discovered with this test.
Usage: ./unit/large_inserts_bugs_stress_test
[--num-inserts <x-Millions>]
[--num-threads 10]
[--verbose-progress]
One test case, currently skipped, caught issue #458.
gapisback
added a commit
that referenced
this issue
Nov 8, 2022
This commit adds a stress test to exercise large #s of inserts,
from a main connection to Splinter, or from multiple threads.
Some test-configuration is provided
o --num-inserts; default 20 Million
o --num-threads default 8, each thread inserts --num-insert KV-pairs
All combinations of seq/random keys/value-pairs are supported.
Currently, no new bugs were discovered with this test.
Usage: ./unit/large_inserts_bugs_stress_test
[--num-inserts <x-Millions>]
[--num-threads 10]
[--verbose-progress]
One test case, currently skipped, caught issue #458.
|
The specific assertion failure unearthed by this new test case has already been fixed in An attempt was made to re-run this new test case with latest code from So, closing this issue out and will track the test-case stabilization and eventually hopefully integration to |
gapisback
added a commit
that referenced
this issue
Mar 24, 2023
- Minor off-by-1 fix in btree_pack_can_fit_tuple() - Fix page-addr computation in routing_filter_prefetch() Test moves along further, but still runs into other failures.
gapisback
added a commit
that referenced
this issue
Mar 29, 2023
…serts() to run cleanly. Some attempted fixes to get test case to run cleanly: test_seq_htobe32_key_random_6byte_values_inserts() - Minor off-by-1 fix in btree_pack_can_fit_tuple() - Fix page-addr computation in routing_filter_prefetch() Test moves along further, but still runs into other failures.
gapisback
added a commit
that referenced
this issue
Mar 29, 2023
gapisback
added a commit
that referenced
this issue
Mar 29, 2023
gapisback
added a commit
that referenced
this issue
Apr 4, 2023
(#458) Attempted fixes to get test_issue_458_mini_destroy_unused_debug_assert() to run cleanly. Test case was written to repro issue #458, but is still unstable. Some attempted fixes to get it to run cleanly. - Minor off-by-1 fix in btree_pack_can_fit_tuple() - Fix page-addr computation in routing_filter_prefetch() Test moves along further, but still runs into other failures. Re-enable test_seq_key_seq_values_inserts_threaded(); seems to work. Revisit. Add test_seq_htobe32_key_random_6byte_values_inserts() to repro PG-assertion. Add a new test case to load int32 key stored in htobe32() format, with a randomly generated 6-byte value. This reproduces the segv (release build) and assertion seen in Postgres benchmarking. Consistently use CTEST_LOG_INFO() rather than platform_default_log() Skip some tests that are failing. Revert fix in btree_pack_can_fit_tuple() - clang-format fixes. Change debug_assert() to platform_assert()
gapisback
added a commit
that referenced
this issue
Apr 4, 2023
(#458) Attempted fixes to get test_issue_458_mini_destroy_unused_debug_assert() to run cleanly. Test case was written to repro issue #458, but is still unstable. Some attempted fixes to get it to run cleanly. - Minor off-by-1 fix in btree_pack_can_fit_tuple() - Fix page-addr computation in routing_filter_prefetch() Test moves along further, but still runs into other failures. Re-enable test_seq_key_seq_values_inserts_threaded(); seems to work. Revisit. Add test_seq_htobe32_key_random_6byte_values_inserts() to repro PG-assertion. Add a new test case to load int32 key stored in htobe32() format, with a randomly generated 6-byte value. This reproduces the segv (release build) and assertion seen in Postgres benchmarking. Consistently use CTEST_LOG_INFO() rather than platform_default_log() Skip some tests that are failing. Revert fix in btree_pack_can_fit_tuple() - clang-format fixes. Change debug_assert() to platform_assert() - Test has been renamed to large_inserts_stress_test
gapisback
added a commit
that referenced
this issue
Apr 6, 2023
(#458) Attempted fixes to get test_issue_458_mini_destroy_unused_debug_assert() to run cleanly. Test case was written to repro issue #458, but is still unstable. Some attempted fixes to get it to run cleanly. - Minor off-by-1 fix in btree_pack_can_fit_tuple() - Fix page-addr computation in routing_filter_prefetch() Test moves along further, but still runs into other failures. Re-enable test_seq_key_seq_values_inserts_threaded(); seems to work. Revisit. Add test_seq_htobe32_key_random_6byte_values_inserts() to repro PG-assertion. Add a new test case to load int32 key stored in htobe32() format, with a randomly generated 6-byte value. This reproduces the segv (release build) and assertion seen in Postgres benchmarking. Consistently use CTEST_LOG_INFO() rather than platform_default_log() Skip some tests that are failing. Revert fix in btree_pack_can_fit_tuple() - clang-format fixes. Change debug_assert() to platform_assert() - Test has been renamed to large_inserts_stress_test
gapisback
added a commit
that referenced
this issue
Apr 11, 2023
…int object mgmt. Stabilize large inserts stress tests. Skip some failing cases. (#458) Attempted fixes to get test_issue_458_mini_destroy_unused_debug_assert() to run cleanly. Test case was written to repro issue #458, but is still unstable. Some attempted fixes to get it to run cleanly. - Minor off-by-1 fix in btree_pack_can_fit_tuple() - Fix page-addr computation in routing_filter_prefetch() Test moves along further, but still runs into other failures. Re-enable test_seq_key_seq_values_inserts_threaded(); seems to work. Revisit. Add test_seq_htobe32_key_random_6byte_values_inserts() to repro PG-assertion. Add a new test case to load int32 key stored in htobe32() format, with a randomly generated 6-byte value. This reproduces the segv (release build) and assertion seen in Postgres benchmarking. Consistently use CTEST_LOG_INFO() rather than platform_default_log() Skip some tests that are failing. Revert fix in btree_pack_can_fit_tuple() - clang-format fixes. Change debug_assert() to platform_assert() - Test has been renamed to large_inserts_stress_test -- Add more diags to track shm-free calls. Rework large inserts stress test cases. This commit is an expedition to triage better shmem OOMs that we are routinely seeing in few of the test cases in large_inserts_stress_test.c . These used to work just fine with 4 GiB shmem in dev branch, but now even with 8 GiB shmem we are running out of memory. Some tracing changes are added to pass-down func/line# where alloc / free is occurring from. And to better report the call-site-info where large-fragments' handling occurs. No real logic change; tests are still unstable. Few cases are marked SKIP, to reduce noise during debugging. -- Minor rework of alloc/free interfaces to use PROCESS_PRIVATE_HEAP_ID -- Change free methods to track 'size' of memory chunk being freed. Plumb thru various free()-related interfaces tracking the 'size' of the memory fragment being freed. Track distribution of # of free calls for memory fragments of diff sizes. Enhance OOM-diags to report these metrics. We can now see that large # of small <= 32-byte fragments are being allocated and freed. Fix platform_realloc() interface, to return new size of aligned memory fragment. This way, when writable_buffer_ensure_space() invokes it, we do proper tracking of the 64-byte memory fragment, rather than mis-tracking it as a 32-byte fragment. - Fix bug in releasing memory from task_system. - Correct couple of uses of platform_free() to now use _free_mem(). - Fix one more case of free_mem() for fingerprint array. -- Implement small free fragment recycling. Add tracing. Use mutex v/s spinlock. This commit adds support for tracking small free fragments in lists, with a fragment header tracking {next, size} info. Add diagnostics to track free fragment usage metrics. Change from use of spinlock to a mutex to protect shared memory stats / lists mgmt. (Otherwise, for multi-threaded test runs, we were running into CPU pegged situation.) -- Suppress assert on size > 0 in platform_shm_track_free() -- Add platform_mem_frag{}, passed to platform_free() to free memory objects This commit starts to bring back the interface to platform_free() and keep platform_free_mem() as an internal interface. Callers that allocate memory fragment for an opaque type, say an array of n-bytes, will now have to package their memory fragment's info in a new platform_mem_frag{} struct. platform_free() is enhanced to recognize an object of this type, and will do the necessary unpacking to call the lower free methods. Fix a bug in trunk_unmount() to free memory for stats-fragments. -- Percolate errors from shared memory destroy all the way to splinterdb_close() As part of dismantling the shared segment, we [already] check to see if there are "un-freed" large-fragments lying around. This indicates some code-flow / logic error in use of platform_free() or platform_free_mem() interfaces. This commit adds the plumbing to percolate this error condition all the way up the top to splinterdb_close(). That has been changed from a void function to one returning platform_status. Other minor fixes, comment cleanup. -- Update few unit-tests to check for rc from splinterdb_close(). -- Re-enable few cases in large_inserts_stress_test. Bump up cache to 4GiB, drop shmem to 1GiB -- Fix to get large_inserts_stress test to run in MSAN builds. -- Implement fingerprint object mgmt apis. Test seems to work. -- Fix call to realloc() that trips assertions. Tighten tests to check rc. Update few unit-tests to check rc from platform_heap_destroy(). If any test does not dismantle sub-systems, test run with --use-shmem will trip up as platform_heap_destroy() will return a failure. -- Fix failing unit-tests. Tighten platform_free() to require non-NULL ptr to free. -- Make fp_array{} a derived object from platform_mem_frag{} To eliminate use of platform_free_mem() interface, define fingerprint fp_array{} as an object extended from a memory fragment. Rework interfaces. Add alias refcount diagnostics. -- Rename to platform_memfrag(). Add more tracing. Fix bugs. -- Purge all refs to platform_free_mem(). Replace with mem_frag{}. Needs to be tested / debugged. -- Rework stats-gathering struct. Fix one debug tests bug. -- Add repro test_fp_num_tuples_out_of_bounds_bug_trunk_build_filters() and instrumentation. Fix failing assert from trunk_build_filters(); Code reorgn issue. -- Fix frags_inuse and frags_inuse_HWM accounting bugs. -- Fix assertion failure in trunk_build_filters() -> fingerprint_unalias(). Consolidate outputs when asserts in fingerprint methods trip.
gapisback
added a commit
that referenced
this issue
Apr 12, 2023
…int object mgmt. Stabilize large inserts stress tests. Skip some failing cases. (#458) Attempted fixes to get test_issue_458_mini_destroy_unused_debug_assert() to run cleanly. Test case was written to repro issue #458, but is still unstable. Some attempted fixes to get it to run cleanly. - Minor off-by-1 fix in btree_pack_can_fit_tuple() - Fix page-addr computation in routing_filter_prefetch() Test moves along further, but still runs into other failures. Re-enable test_seq_key_seq_values_inserts_threaded(); seems to work. Revisit. Add test_seq_htobe32_key_random_6byte_values_inserts() to repro PG-assertion. Add a new test case to load int32 key stored in htobe32() format, with a randomly generated 6-byte value. This reproduces the segv (release build) and assertion seen in Postgres benchmarking. Consistently use CTEST_LOG_INFO() rather than platform_default_log() Skip some tests that are failing. Revert fix in btree_pack_can_fit_tuple() - clang-format fixes. Change debug_assert() to platform_assert() - Test has been renamed to large_inserts_stress_test -- Add more diags to track shm-free calls. Rework large inserts stress test cases. This commit is an expedition to triage better shmem OOMs that we are routinely seeing in few of the test cases in large_inserts_stress_test.c . These used to work just fine with 4 GiB shmem in dev branch, but now even with 8 GiB shmem we are running out of memory. Some tracing changes are added to pass-down func/line# where alloc / free is occurring from. And to better report the call-site-info where large-fragments' handling occurs. No real logic change; tests are still unstable. Few cases are marked SKIP, to reduce noise during debugging. -- Minor rework of alloc/free interfaces to use PROCESS_PRIVATE_HEAP_ID -- Change free methods to track 'size' of memory chunk being freed. Plumb thru various free()-related interfaces tracking the 'size' of the memory fragment being freed. Track distribution of # of free calls for memory fragments of diff sizes. Enhance OOM-diags to report these metrics. We can now see that large # of small <= 32-byte fragments are being allocated and freed. Fix platform_realloc() interface, to return new size of aligned memory fragment. This way, when writable_buffer_ensure_space() invokes it, we do proper tracking of the 64-byte memory fragment, rather than mis-tracking it as a 32-byte fragment. - Fix bug in releasing memory from task_system. - Correct couple of uses of platform_free() to now use _free_mem(). - Fix one more case of free_mem() for fingerprint array. -- Implement small free fragment recycling. Add tracing. Use mutex v/s spinlock. This commit adds support for tracking small free fragments in lists, with a fragment header tracking {next, size} info. Add diagnostics to track free fragment usage metrics. Change from use of spinlock to a mutex to protect shared memory stats / lists mgmt. (Otherwise, for multi-threaded test runs, we were running into CPU pegged situation.) -- Suppress assert on size > 0 in platform_shm_track_free() -- Add platform_mem_frag{}, passed to platform_free() to free memory objects This commit starts to bring back the interface to platform_free() and keep platform_free_mem() as an internal interface. Callers that allocate memory fragment for an opaque type, say an array of n-bytes, will now have to package their memory fragment's info in a new platform_mem_frag{} struct. platform_free() is enhanced to recognize an object of this type, and will do the necessary unpacking to call the lower free methods. Fix a bug in trunk_unmount() to free memory for stats-fragments. -- Percolate errors from shared memory destroy all the way to splinterdb_close() As part of dismantling the shared segment, we [already] check to see if there are "un-freed" large-fragments lying around. This indicates some code-flow / logic error in use of platform_free() or platform_free_mem() interfaces. This commit adds the plumbing to percolate this error condition all the way up the top to splinterdb_close(). That has been changed from a void function to one returning platform_status. Other minor fixes, comment cleanup. -- Update few unit-tests to check for rc from splinterdb_close(). -- Re-enable few cases in large_inserts_stress_test. Bump up cache to 4GiB, drop shmem to 1GiB -- Fix to get large_inserts_stress test to run in MSAN builds. -- Implement fingerprint object mgmt apis. Test seems to work. -- Fix call to realloc() that trips assertions. Tighten tests to check rc. Update few unit-tests to check rc from platform_heap_destroy(). If any test does not dismantle sub-systems, test run with --use-shmem will trip up as platform_heap_destroy() will return a failure. -- Fix failing unit-tests. Tighten platform_free() to require non-NULL ptr to free. -- Make fp_array{} a derived object from platform_mem_frag{} To eliminate use of platform_free_mem() interface, define fingerprint fp_array{} as an object extended from a memory fragment. Rework interfaces. Add alias refcount diagnostics. -- Rename to platform_memfrag(). Add more tracing. Fix bugs. -- Purge all refs to platform_free_mem(). Replace with mem_frag{}. Needs to be tested / debugged. -- Rework stats-gathering struct. Fix one debug tests bug. -- Add repro test_fp_num_tuples_out_of_bounds_bug_trunk_build_filters() and instrumentation. Fix failing assert from trunk_build_filters(); Code reorgn issue. -- Fix frags_inuse and frags_inuse_HWM accounting bugs. -- Fix assertion failure in trunk_build_filters() -> fingerprint_unalias(). Consolidate outputs when asserts in fingerprint methods trip. -- Rework fingerprint unalias'ing in trunk_bundle_build_filters(); clang compile fails w/ older code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
While trying to setup a manual unit-test to repro issue #457, ran into the following stack using debug build of Splinter:
The extended assertion message reports:
Inserted 28 million KV-pairs ...
Assertion failed at src/mini_allocator.c:702:mini_destroy_unused(): "(mini->num_extents == mini->num_batches)". num_extents=9, num_batches=8
Reproduction steps
Updated (gapisback):
To help move the fix/dev along, I pushed this branch: agurajada/458-mini_destroy_unused-Assert
Containing a new stress test: large_inserts_bugs_stress_test.c. It's pretty simple but useful to repro this problem with debug builds.
...
Expected behavior
Assertion should not occur.
Additional context
From code inspection,
mini->num_batchesseems to be fixed whenbtree_createcallsmini_init, here:BTREE_MAX_HEIGHTis #defined to 8, which is consistent with the value ofnum_batches=8seen when the assert trips.The text was updated successfully, but these errors were encountered: