From 2e12cd2492c147d826003bb5f821b9b425ee2a77 Mon Sep 17 00:00:00 2001 From: Nalini Ganapati <35076948+nalinigans@users.noreply.github.com> Date: Tue, 26 Nov 2024 19:32:46 -0800 Subject: [PATCH] Fix memory leaks, add parallel examples for testing and support valgrind githib actions (#179) * Fix memory leaks, add parallel examples for testing and support valgrind githib actions --- .github/workflows/valgrind.yml | 96 +++++++++++++++++++++++++++++ CMakeLists.txt | 2 +- core/src/array/array.cc | 1 + core/src/storage/storage_manager.cc | 4 -- examples/CMakeLists.txt | 5 +- examples/expected_results_parallel | 42 +++++++++++++ examples/run_examples.sh | 33 +--------- examples/run_examples_base.sh | 76 +++++++++++++++++++++++ examples/run_examples_parallel.sh | 45 ++++++++++++++ 9 files changed, 265 insertions(+), 39 deletions(-) create mode 100644 .github/workflows/valgrind.yml create mode 100644 examples/expected_results_parallel create mode 100644 examples/run_examples_base.sh create mode 100755 examples/run_examples_parallel.sh diff --git a/.github/workflows/valgrind.yml b/.github/workflows/valgrind.yml new file mode 100644 index 00000000..67509cdb --- /dev/null +++ b/.github/workflows/valgrind.yml @@ -0,0 +1,96 @@ +# +# valgrind.yml +# +# The MIT License +# +# Copyright (c) 2024 dātma, inc™ +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. +# + +name: valgrind + +on: + push: + #branches: [ master, develop ] + paths-ignore: + - '**/*.md' + pull_request: + #branches: [ master, develop ] + paths-ignore: + - '**/*.md' + +env: + CMAKE_BUILD_DIR: ${{github.workspace}}/build + AWSSDK_VER: 1.8.x + GCSSDK_VER: v1.24.0 + +jobs: + build: + + runs-on: ubuntu-22.04 + + steps: + - uses: actions/checkout@v4 + - name: Install Dependencies + run: | + sudo apt-get update -q + sudo apt-get -y install zlib1g-dev libssl-dev uuid-dev libcurl4-openssl-dev + sudo apt-get -y install valgrind + sudo apt-get -y install catch2 + + - name: Cache AWS SDK + uses: actions/cache@v4 + with: + path: ~/awssdk-install + key: awssdk-${{env.AWSSDK_VER}}-${{env.ImageOS}}-${{env.ImageVersion}}-openssl-3-v1 + + - name: Cache GCS SDK + uses: actions/cache@v4 + with: + path: ~/gcssdk-install + key: gcssdk-${{env.GCSSDK_VER}}-${{env.ImageOS}}-${{env.ImageVersion}}-openssl-3-v1 + + - name: Run valgrind + shell: bash + run: | + mkdir -p $CMAKE_BUILD_DIR + cd $CMAKE_BUILD_DIR + cmake -DCMAKE_BUILD_TYPE=Debug -DTILEDB_DISABLE_TESTING=1 -DUSE_HDFS=0 -DUSE_OPENMP=0 $GITHUB_WORKSPACE + make -j4 + make examples + cd examples + export VALGRIND="valgrind --leak-check=full" + $VALGRIND ./tiledb_workspace_group_create + $VALGRIND ./tiledb_array_create_sparse + $VALGRIND ./tiledb_array_write_sparse_1 + $VALGRIND ./tiledb_array_write_sparse_2 + $VALGRIND ./tiledb_array_read_sparse_1 + $VALGRIND ./tiledb_array_read_sparse_2 + $VALGRIND ./tiledb_array_read_sparse_filter_1 + $VALGRIND ./tiledb_array_iterator_sparse + $VALGRIND ./tiledb_array_iterator_sparse_filter + $VALGRIND ./tiledb_array_parallel_write_sparse_1 + # tiledb_array_parallel_write_sparse_2 requires OpenMP + # $VALGRIND ./tiledb_array_parallel_write_sparse_2 + $VALGRIND ./tiledb_array_parallel_read_sparse_1 + # tiledb_array_parallel_read_sparse_2 requires OpenMP + # $VALGRIND ./tiledb_array_parallel_read_sparse_2 + $VALGRIND ./tiledb_array_parallel_consolidate_sparse + diff --git a/CMakeLists.txt b/CMakeLists.txt index ababddcc..5ad81f82 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -258,7 +258,7 @@ endif() add_subdirectory(core) # Enable testing -if(NOT TILEDB_DISABLE_TESTING) +if(NOT TILEDB_DISABLE_TESTS) enable_testing() add_custom_target(tests COMMAND ${CMAKE_CTEST_COMMAND} -V) diff --git a/core/src/array/array.cc b/core/src/array/array.cc index d9cacbbd..e4c4ef17 100644 --- a/core/src/array/array.cc +++ b/core/src/array/array.cc @@ -615,6 +615,7 @@ int Array::finalize() { } if (consolidate_mode()) { + free_array_schema(); return fg_error?TILEDB_AR_ERR:TILEDB_AR_OK; } diff --git a/core/src/storage/storage_manager.cc b/core/src/storage/storage_manager.cc index ae3b5b94..c0b03ff5 100644 --- a/core/src/storage/storage_manager.cc +++ b/core/src/storage/storage_manager.cc @@ -1468,11 +1468,7 @@ int StorageManager::array_open( open_array->array_schema_) != TILEDB_SM_OK) return TILEDB_SM_ERR; } - } - if (!array_consolidate_mode(mode)) { - // Load the book-keeping for each fragment. For consolidate mode, each fragment will be opened - // separately if(array_load_book_keeping( open_array->array_schema_, open_array->fragment_names_, diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index de997b5c..6d5d5dca 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -6,7 +6,7 @@ # # Copyright (c) 2016 MIT and Intel Corporation # Copyright (c) 2018-2019 Omics Data Automation, Inc. -# Copyright (c) 2023 dātma, inc™ +# Copyright (c) 2023-2024 dātma, inc™ # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -61,7 +61,8 @@ add_dependencies(examples ${EXAMPLE_BINS}) if(NOT TILEDB_DISABLE_TESTING) set(CI_TESTS ${CMAKE_SOURCE_DIR}/examples/run_examples.sh && echo "Running diff with expected_results" && - diff log ${CMAKE_SOURCE_DIR}/examples/expected_results) + diff log ${CMAKE_SOURCE_DIR}/examples/expected_results && + ${CMAKE_SOURCE_DIR}/examples/run_examples_parallel.sh) add_test(NAME ci_tests COMMAND ${CI_TESTS}) add_custom_target(ci_tests COMMAND ${CI_TESTS} DEPENDS examples) add_dependencies(tests ci_tests) diff --git a/examples/expected_results_parallel b/examples/expected_results_parallel new file mode 100644 index 00000000..69af153d --- /dev/null +++ b/examples/expected_results_parallel @@ -0,0 +1,42 @@ +Example 1: Running ./tiledb_array_parallel_write_dense_1... +Example 1: Done running ./tiledb_array_parallel_write_dense_1 +Example 2: Running ./tiledb_array_parallel_write_dense_2... +Example 2: Done running ./tiledb_array_parallel_write_dense_2 +Example 3: Running ./tiledb_array_parallel_write_sparse_1... +Example 3: Done running ./tiledb_array_parallel_write_sparse_1 +Example 4: Running ./tiledb_array_parallel_write_sparse_2... +Example 4: Done running ./tiledb_array_parallel_write_sparse_2 +Example 5: Running ./tiledb_array_parallel_read_dense_1... +Number of a1 values greater than 10: 5 +Example 5: Done running ./tiledb_array_parallel_read_dense_1 +Example 6: Running ./tiledb_array_parallel_read_dense_2... +Number of a1 values greater than 10: 5 +Example 6: Done running ./tiledb_array_parallel_read_dense_2 +Example 7: Running ./tiledb_array_parallel_read_sparse_1... +Number of a1 values greater than 5: 4 +Example 7: Done running ./tiledb_array_parallel_read_sparse_1 +Example 8: Running ./tiledb_array_parallel_read_sparse_2... +Number of a1 values greater than 5: 2 +Example 8: Done running ./tiledb_array_parallel_read_sparse_2 +Example 9: Running ./tiledb_array_parallel_consolidate_dense... +Started consolidation +Started reading +Finished reading +Started reading +Started reading +Finished reading +Finished reading +Started reading +Finished reading +Finished consolidation +Number of a1 values greater than 10: 5 +Example 9: Done running ./tiledb_array_parallel_consolidate_dense +Example 10: Running ./tiledb_array_parallel_consolidate_sparse... +Started consolidation +Started reading +Started reading +Finished reading +Finished reading +Finished consolidation +Number of a1 values greater than 5: 4 +Example 10: Done running ./tiledb_array_parallel_consolidate_sparse diff --git a/examples/run_examples.sh b/examples/run_examples.sh index 265ef302..64e56dbd 100755 --- a/examples/run_examples.sh +++ b/examples/run_examples.sh @@ -39,38 +39,7 @@ # e.g for ./run_examples.sh gs://my_bucket/my_dir/my_test, expect results in test.log # Check the log file against the /examples/expected_results file. -check_rc() { - if [[ $# -eq 1 ]]; then - if [[ $1 -ne 0 ]]; then - echo - echo "Exit Status=$1. Quitting execution of run_examples.sh" - exit $1 - fi - fi -} - -run_example() { - if [[ $# -eq 3 ]] - then - logfile=`basename $2`.log - echo "Example $3: Running $1..." | tee -a ${logfile} - $1 $2 | tee -a ${logfile} - check_rc ${PIPESTATUS[0]} - echo "Example $3: Done running $1" | tee -a ${logfile} - else - echo "Example $2: Running $1..." | tee -a log - $1 | tee -a log - check_rc ${PIPESTATUS[0]} - echo "Example $2: Done running $1" | tee -a log - fi -} - -if [[ -n $1 ]] -then - rm -fr `basename $1`.log -else - rm -fr log -fi +source $(dirname $0)/run_examples_base.sh "$@" run_example ./tiledb_workspace_group_create $1 1 run_example ./tiledb_ls_workspaces $1 2 diff --git a/examples/run_examples_base.sh b/examples/run_examples_base.sh new file mode 100644 index 00000000..8a1b990e --- /dev/null +++ b/examples/run_examples_base.sh @@ -0,0 +1,76 @@ +#!/bin/bash +# +# run_examples_base.sh +# +# +# The MIT License +# +# Copyright (c) 2024 dātma, inc™ +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. +# + + +check_rc() { + if [[ $# -eq 1 ]]; then + if [[ $1 -ne 0 ]]; then + echo + echo "Exit Status=$1. Quitting execution of run_examples.sh" + exit $1 + fi + fi +} + +log_filename() { + if [[ $# -ge 1 ]] + then + if [[ $(basename $0) == "run_examples_parallel.sh" ]] + then + export LOGFILENAME=`basename $1`.parallel.log + else + export LOGFILENAME=`basename $1`.log + fi + else + if [[ $(basename $0) == "run_examples_parallel.sh" ]] + then + export LOGFILENAME=parallel.log + else + export LOGFILENAME=log + fi + fi +} + +run_example() { + if [[ $# -eq 3 ]] + then + logfile=`basename $2`.log + echo "Example $3: Running $1..." | tee -a $LOGFILENAME + $1 $2 | tee -a $LOGFILENAME + check_rc ${PIPESTATUS[0]} + echo "Example $3: Done running $1" | tee -a $LOGFILENAME + else + echo "Example $2: Running $1..." | tee -a $LOGFILENAME + $1 | tee -a $LOGFILENAME + check_rc ${PIPESTATUS[0]} + echo "Example $2: Done running $1" | tee -a $LOGFILENAME + fi +} + +log_filename $@ +rm -f $LOGFILENAME diff --git a/examples/run_examples_parallel.sh b/examples/run_examples_parallel.sh new file mode 100755 index 00000000..4010bd15 --- /dev/null +++ b/examples/run_examples_parallel.sh @@ -0,0 +1,45 @@ +#!/bin/bash +# +# run_examples_base.sh +# +# +# The MIT License +# +# Copyright (c) 2024 dātma, inc™ +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. +# + +source $(dirname $0)/run_examples_base.sh "$@" + +run_example ./tiledb_array_parallel_write_dense_1 $1 1 +run_example ./tiledb_array_parallel_write_dense_2 $1 2 +run_example ./tiledb_array_parallel_write_sparse_1 $1 3 +run_example ./tiledb_array_parallel_write_sparse_2 $1 4 +run_example ./tiledb_array_parallel_read_dense_1 $1 5 +run_example ./tiledb_array_parallel_read_dense_2 $1 6 +run_example ./tiledb_array_parallel_read_sparse_1 $1 7 +run_example ./tiledb_array_parallel_read_sparse_2 $1 8 +run_example ./tiledb_array_parallel_consolidate_dense $1 9 +run_example ./tiledb_array_parallel_consolidate_sparse $1 10 + +# Cannot do a diff for parallel tests as there is no deterministic order +# for the results, so just run them +# diff $LOGFILENAME $(dirname $0)/expected_results_parallel +