Stabilization changes for platform_free() and free-list fragment management. #565
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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.
…()'s tight checks.
✅ Deploy Preview for splinterdb canceled.
|
When attempting to free a large-fragment that is of the wrong size, we may not find the tracked large free-fragment. debug_assert()..
…nt_alias_unalias_deinit
gapisback
force-pushed
the
agurajada/shmem-free2
branch
from
7ee3fe8 to
5cd8648
Compare
- Purge splinter_shm_realloc(), splinter_shm_free() etc wrappers. No value-add. - Rework test cases and comments in splinter_shmem_test.c to separate out usage of 'realloc' v/s 'reuse'
This fixds a small shmem-related bug in call to platform_realloc() by writable_buffer_ensure_space().
Add fix to realloc() interfaces and also to shm_realloc(). Add test_large_frag_platform_realloc_to_large_frag. Fix bug in test code in call to platform_free().
gapisback
force-pushed
the
agurajada/shmem-free2
branch
from
175e4de to
14469d3
Compare
Extend test.sh to include fast-running splinter_shmem_test and also platform_apis_test. This gives extended coverage for fast tests w/ and w/o --use-shmem.
|
These dev change-set changes have been applied to the final version of part-3 support for shared memory. PR #569 is now ready for review, so this temp dev work can now be deleted. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.