Merge branch 'master' of github.com:man-group/ArcticDB into ostoimeno…

…v/performance_timestamp
man-group · Jan 6, 2025 · 0e6a1e3 · 0e6a1e3
2 parents 3aa26ac + a28bf7f
commit 0e6a1e3
Show file tree

Hide file tree

Showing 15 changed files with 195 additions and 76 deletions.
diff --git a/.github/actions/setup_deps/action.yml b/.github/actions/setup_deps/action.yml
diff --git a/.github/workflows/analysis_workflow.yml b/.github/workflows/analysis_workflow.yml
@@ -5,6 +5,10 @@ on:
       run_all_benchmarks:
         type: boolean
         default: false
+      dev_image_tag:
+        description: Tag of the ArcticCB development image to use for benchmark and code coverage flows
+        type: string
+        default: latest
 
   schedule: # Schedule the job to run at 12 a.m. daily
     - cron: '0 0 * * *'
@@ -45,6 +49,7 @@ jobs:
       commit: ${{ matrix.commits }}
       run_all_benchmarks: ${{ inputs.run_all_benchmarks || false }}
       run_on_pr_head: ${{ github.event_name == 'pull_request_target' }}
+      dev_image_tag: ${{ inputs.dev_image_tag || 'latest' }}
 
   publish_benchmark_results_to_gh_pages:  
     name: Publish benchmark results to gh-pages

diff --git a/.github/workflows/benchmark_commits.yml b/.github/workflows/benchmark_commits.yml
@@ -5,6 +5,7 @@ on:
       run_all_benchmarks: {required: true, type: boolean, description: Run all benchmarks or just the one for the given commit}
       commit:             {required: true, type: string, description: commit hash that will be benchmarked}
       run_on_pr_head:     {required: false, default: false, type: boolean, description: Specifies if the benchmark should run on PR head branch}
+      dev_image_tag:      {required: false, default: 'latest', type: string, description: Tag of the ArcticDB development image}
 jobs:
   start_ec2_runner:
     uses: ./.github/workflows/ec2_runner_jobs.yml
@@ -19,7 +20,7 @@ jobs:
       always() &&
       !cancelled()
     runs-on: ${{ needs.start_ec2_runner.outputs.label }}
-    container: quay.io/pypa/manylinux_2_28_x86_64:latest
+    container: ghcr.io/man-group/arcticdb-dev:${{ inputs.dev_image_tag }}
     env:
       # this is potentially overflowing the cache, so should be looked into after we address issue #1057
       SCCACHE_GHA_VERSION: ${{vars.SCCACHE_GHA_VERSION || 1}} # Setting this env var enables the caching
@@ -31,11 +32,6 @@ jobs:
     defaults:
       run: {shell: bash}
     steps:  
-      - name: Initialize LFS
-        shell: bash -l {0}
-        run: |
-          dnf install -y git-lfs
-
       - uses: actions/[email protected]
         with:
           lfs: 'true'
@@ -48,9 +44,6 @@ jobs:
         uses: mozilla-actions/[email protected]
         with:
           version: "v0.4.0"
-
-      - name: Install deps
-        uses: ./.github/actions/setup_deps
 
       - name: Extra envs
         shell: bash -l {0}

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -19,6 +19,10 @@ on:
         description: Override CMAKE preset type
         type: choice
         options: ["-", debug, release]
+      dev_image_tag:
+        description: Tag of the ArcticCB development image to use for the Linux C++ tests build
+        type: string
+        default: latest
 run-name: Building ${{github.ref_name}} on ${{github.event_name}} by ${{github.actor}}
 concurrency:
   group: ${{github.ref}}
@@ -52,7 +56,7 @@ jobs:
                   mongodb:
                     image: "mongo:4.4"
                 container:
-                  image: quay.io/pypa/manylinux_2_28_x86_64
+                  image: ghcr.io/man-group/arcticdb-dev:${{inputs.dev_image_tag || 'latest'}}
                   volumes:
                     - /:/mnt
             windows_matrix:

diff --git a/.github/workflows/build_steps.yml b/.github/workflows/build_steps.yml
@@ -87,10 +87,6 @@ jobs:
           maximum-size: 6GB
           disk-root: "D:"  # This is also the checkout directory. Total size 12GB.
         continue-on-error: true
-
-      - name: Install deps
-        if: matrix.os == 'linux' && inputs.job_type != 'build-python-wheels'
-        uses: ./.github/actions/setup_deps
 
       - name: Extra envs
         # This has to come after msvc-dev-cmd to overwrite the bad VCPKG_ROOT it sets

diff --git a/.github/workflows/dev_docker_image.yml b/.github/workflows/dev_docker_image.yml
@@ -0,0 +1,43 @@
+name: Docker Image for Development
+on:
+  # should support manual trigger and PRs to the Dockerfile
+  push:
+    paths:
+      - 'docker/**'
+      - '.github/workflows/dev_docker_image.yml'
+  workflow_dispatch:
+
+jobs:
+  build_and_push:
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      packages: write
+    steps:
+      - name: Checkout
+        uses: actions/[email protected]
+
+      - name: Choose image tag
+        run: |
+          sha=$(git rev-parse --short HEAD)
+          image_ver="$(date '+%Y%m%d')-${sha}"
+          image_name="ghcr.io/man-group/arcticdb-dev"
+          echo -e "image_ver=$image_ver\nimage_name=$image_name\noutput_tag=$image_name:$image_ver" | tee -a $GITHUB_ENV
+
+      - name: Build Docker image
+        run: |
+          docker build -t $image_name . -f docker/Dockerfile
+
+      - name: Login to GHCR
+        run: docker login ghcr.io -u token -p "${{secrets.GITHUB_TOKEN}}"
+
+      - name: Publish Docker versioned image to GHCR
+        run: |
+          docker tag $image_name $image_name:$image_ver
+          docker push $image_name:$image_ver
+
+      - name: Publish Docker image to GHCR as latest
+        if: github.ref == 'refs/heads/master'
+        run: |
+          docker tag $image_name $image_name:latest
+          docker push $image_name:latest
diff --git a/cpp/arcticdb/storage/memory_layout.hpp b/cpp/arcticdb/storage/memory_layout.hpp
@@ -101,15 +101,15 @@ enum class BitmapFormat : uint8_t {
 // pointer to the first block
 struct EncodedField {
     EncodedFieldType type_ = EncodedFieldType::UNKNOWN;
-    uint16_t shapes_count_ = 0u;
-    uint16_t values_count_ = 0u;
+    uint32_t shapes_count_ = 0u;
+    uint32_t values_count_ = 0u;
     uint32_t sparse_map_bytes_ = 0u;
     uint32_t items_count_ = 0u;
     BitmapFormat format_ = BitmapFormat::UNKNOWN;
     std::array<Block, 1> blocks_;
 };
 
-static_assert(sizeof(EncodedField) == 60);
+static_assert(sizeof(EncodedField) == 64);
 
 enum class EncodingVersion : uint16_t {
     V1 = 0,

diff --git a/cpp/arcticdb/storage/s3/detail-inl.hpp b/cpp/arcticdb/storage/s3/detail-inl.hpp
@@ -168,6 +168,7 @@ namespace s3 {
         template<class KeyBucketizer>
         void do_read_impl(Composite<VariantKey> &&ks,
                           const ReadVisitor &visitor,
+                          folly::Function<VariantKey(VariantKey&&)> key_decoder,
                           const std::string &root_folder,
                           const std::string &bucket_name,
                           const S3ClientWrapper &s3_client,
@@ -179,7 +180,7 @@ namespace s3 {
 
             (fg::from(ks.as_range()) | fg::move | fg::groupBy(fmt_db)).foreach(
                     [&s3_client, &bucket_name, &root_folder, b = std::move(bucketizer), &visitor, &keys_not_found,
-                            opts = opts](auto &&group) {
+                            &key_decoder, opts = opts](auto &&group) {
 
                         for (auto &k: group.values()) {
                             auto key_type_dir = key_type_folder(root_folder, variant_key_type(k));
@@ -189,25 +190,26 @@ namespace s3 {
                                     s3_object_name,
                                     bucket_name);
 
+                            auto unencoded_key = key_decoder(std::move(k));
                             if (get_object_result.is_success()) {
                                 ARCTICDB_SUBSAMPLE(S3StorageVisitSegment, 0)
 
-                                visitor(k, std::move(get_object_result.get_output()));
+                                visitor(unencoded_key, std::move(get_object_result.get_output()));
 
-                                ARCTICDB_DEBUG(log::storage(), "Read key {}: {}", variant_key_type(k),
-                                               variant_key_view(k));
+                                ARCTICDB_DEBUG(log::storage(), "Read key {}: {}", variant_key_type(unencoded_key),
+                                               variant_key_view(unencoded_key));
                             } else {
                                 auto &error = get_object_result.get_error();
                                 raise_if_unexpected_error(error, s3_object_name);
 
                                 log::storage().log(
                                     opts.dont_warn_about_missing_key ? spdlog::level::debug : spdlog::level::warn,
                                     "Failed to find segment for key '{}' {}: {}",
-                                    variant_key_view(k),
+                                    variant_key_view(unencoded_key),
                                     error.GetExceptionName().c_str(),
                                     error.GetMessage().c_str());
 
-                                keys_not_found.push_back(k);
+                                keys_not_found.push_back(unencoded_key);
                             }
                         }
                     });

diff --git a/cpp/arcticdb/storage/s3/nfs_backed_storage.cpp b/cpp/arcticdb/storage/s3/nfs_backed_storage.cpp
@@ -171,15 +171,11 @@ void NfsBackedStorage::do_update(Composite<KeySegmentPair>&& kvs, UpdateOpts) {
 }
 
 void NfsBackedStorage::do_read(Composite<VariantKey>&& ks, const ReadVisitor& visitor, ReadKeyOpts opts) {
-    auto func = [visitor] (const VariantKey& k, Segment&& seg) mutable {
-        visitor(unencode_object_id(k), std::move(seg));
-    };
-
     auto enc = ks.transform([] (auto&& key) {
         return encode_object_id(key);
     });
 
-    s3::detail::do_read_impl(std::move(enc), func, root_folder_, bucket_name_, *s3_client_, NfsBucketizer{}, opts);
+    s3::detail::do_read_impl(std::move(enc), visitor, unencode_object_id, root_folder_, bucket_name_, *s3_client_, NfsBucketizer{}, opts);
 }
 
 void NfsBackedStorage::do_remove(Composite<VariantKey>&& ks, RemoveOpts) {

diff --git a/cpp/arcticdb/storage/s3/s3_storage.cpp b/cpp/arcticdb/storage/s3/s3_storage.cpp
@@ -62,7 +62,7 @@ void S3Storage::do_update(Composite<KeySegmentPair>&& kvs, UpdateOpts) {
 }
 
 void S3Storage::do_read(Composite<VariantKey>&& ks, const ReadVisitor& visitor, ReadKeyOpts opts) {
-    detail::do_read_impl(std::move(ks), visitor, root_folder_, bucket_name_, *s3_client_, FlatBucketizer{}, opts);
+    detail::do_read_impl(std::move(ks), visitor, folly::identity, root_folder_, bucket_name_, *s3_client_, FlatBucketizer{}, opts);
 }
 
 void S3Storage::do_remove(Composite<VariantKey>&& ks, RemoveOpts) {

diff --git a/cpp/arcticdb/storage/test/test_s3_storage.cpp b/cpp/arcticdb/storage/test/test_s3_storage.cpp
@@ -11,6 +11,7 @@
 #include <arcticdb/storage/s3/s3_api.hpp>
 #include <arcticdb/storage/s3/s3_storage.hpp>
 #include <arcticdb/storage/s3/s3_mock_client.hpp>
+#include <arcticdb/storage/s3/nfs_backed_storage.hpp>
 #include <arcticdb/storage/s3/detail-inl.hpp>
 #include <arcticdb/entity/protobufs.hpp>
 #include <arcticdb/entity/variant_key.hpp>
@@ -225,7 +226,38 @@ class S3StorageFixture : public testing::Test {
     S3Storage store;
 };
 
-TEST_F(S3StorageFixture, test_key_exists){
+arcticdb::storage::nfs_backed::NfsBackedStorage::Config get_test_nfs_config() {
+    arcticdb::storage::nfs_backed::NfsBackedStorage::Config cfg;
+    cfg.set_use_mock_storage_for_testing(true);
+    return cfg;
+}
+
+class NfsStorageFixture : public testing::Test {
+protected:
+    NfsStorageFixture():
+        store(LibraryPath("lib", '.'), OpenMode::DELETE, get_test_nfs_config())
+    {}
+
+    arcticdb::storage::nfs_backed::NfsBackedStorage store;
+};
+
+class S3AndNfsStorageFixture : public testing::TestWithParam<std::string> {
+public:
+    std::unique_ptr<Storage> get_storage() {
+        LibraryPath lp{"lib"};
+        if (GetParam() == "nfs") {
+            return std::make_unique<arcticdb::storage::nfs_backed::NfsBackedStorage>(
+                lp, OpenMode::DELETE, get_test_nfs_config());
+        } else if (GetParam() == "s3") {
+            return std::make_unique<S3Storage>(
+                lp, OpenMode::DELETE, S3Settings(get_test_s3_config()));
+        } else {
+            util::raise_rte("Unexpected fixture type {}", GetParam());
+        }
+    }
+};
+
+TEST_F(S3StorageFixture, test_key_exists) {
     write_in_store(store, "symbol");
 
     ASSERT_TRUE(exists_in_store(store, "symbol"));
@@ -245,6 +277,23 @@ TEST_F(S3StorageFixture, test_read){
         UnexpectedS3ErrorException);
 }
 
+TEST_P(S3AndNfsStorageFixture, test_read_missing_key_in_exception){
+    auto s = get_storage();
+    auto& store = *s;
+
+    try {
+        read_in_store(store, "snap-not-present", KeyType::SNAPSHOT_REF);
+        FAIL();
+    } catch (KeyNotFoundException& e) {
+        auto keys = e.keys().as_range();
+        ASSERT_EQ(keys.size(), 1);
+        const auto& key = keys.at(0);
+        ASSERT_EQ(variant_key_id(key), StreamId{"snap-not-present"});
+    }
+}
+
+INSTANTIATE_TEST_SUITE_P(S3AndNfs, S3AndNfsStorageFixture, testing::Values("s3", "nfs"));
+
 TEST_F(S3StorageFixture, test_write){
     write_in_store(store, "symbol");
     ASSERT_THROW(

diff --git a/cpp/arcticdb/version/snapshot.cpp b/cpp/arcticdb/version/snapshot.cpp
@@ -107,7 +107,9 @@ void iterate_snapshots(const std::shared_ptr<Store>& store, folly::Function<void
             visitor(vk);
         } catch (storage::KeyNotFoundException& e) {
             e.keys().broadcast([&vk, &e](const VariantKey& key) {
-                if (key != vk) throw storage::KeyNotFoundException(std::move(e.keys()));
+                if (key != vk) {
+                    throw storage::KeyNotFoundException(std::move(e.keys()));
+                }
             });
             ARCTICDB_DEBUG(log::version(), "Ignored exception due to {} being deleted during iterate_snapshots().");
         }

diff --git a/docker/Dockerfile b/docker/Dockerfile
@@ -0,0 +1,26 @@
+FROM quay.io/pypa/manylinux_2_28_x86_64
+
+RUN dnf update -y
+RUN dnf remove -y 'gcc-toolset-*'
+RUN dnf install -y zip flex bison gcc-toolset-10 gcc-toolset-10-gdb gcc-toolset-10-libatomic-devel krb5-devel cyrus-sasl-devel openssl-devel \
+unzip tar epel-release jq wget libcurl-devel git-lfs \
+python3.11-devel python3.11-pip perl-IPC-Cmd
+
+RUN dnf groupinstall -y 'Development Tools'
+RUN dnf install -y mono-complete
+
+RUN dnf clean all
+
+RUN wget -nv https://github.com/mozilla/sccache/releases/download/v0.8.2/sccache-v0.8.2-x86_64-unknown-linux-musl.tar.gz
+RUN tar xvf sccache*.tar.gz
+RUN mv sccache-*/sccache .
+RUN chmod 555 sccache
+
+RUN cp sccache /usr/local/bin/
+
+ENV CC=/opt/rh/gcc-toolset-10/root/bin/gcc
+ENV CMAKE_C_COMPILER=/opt/rh/gcc-toolset-10/root/bin/gcc
+ENV CXX=/opt/rh/gcc-toolset-10/root/bin/g++
+ENV CMAKE_CXX_COMPILER=/opt/rh/gcc-toolset-10/root/bin/g++
+ENV LD_LIBRARY_PATH=/opt/rh/gcc-toolset-10/root/usr/lib64:/opt/rh/gcc-toolset-10/root/usr/lib:/opt/rh/gcc-toolset-10/root/usr/lib64/dyninst
+ENV PATH=/opt/rh/devtoolset-10/root/usr/bin:/opt/python/cp311-cp311/bin:${PATH}
diff --git a/python/tests/hypothesis/arcticdb/test_resample.py b/python/tests/hypothesis/arcticdb/test_resample.py
@@ -54,7 +54,7 @@ def freq_fits_in_64_bits(count, unit):
 
 @st.composite
 def rule(draw):
-    count = draw(st.integers(min_value=1))
+    count = draw(st.integers(min_value=1, max_value=10_000))
     unit = draw(st.sampled_from(['min', 'h']))
     result = f"{count}{unit}"
     assume(freq_fits_in_64_bits(count=count, unit=unit))
@@ -65,7 +65,7 @@ def offset(draw):
     unit = draw(st.sampled_from(['s', 'min', 'h', None]))
     if unit is None:
         return None
-    count = draw(st.integers(min_value=1))
+    count = draw(st.integers(min_value=1, max_value=10_000))
     result = f"{count}{unit}"
     assume(freq_fits_in_64_bits(count=count, unit=unit))
     return result