Apply a few more updates

.github/workflows/daily_benchmark.yaml

@@ -75,8 +75,6 @@ jobs:
 
           ./benchmark.py vendor-native --num-workers-for-benchmark 12 --export-results cartesian.json cartesian
 
-          ./benchmark.py vendor-native --num-workers-for-benchmark 12 --export-results order_by.json order_by
-
       - name: Upload mgbench results
         run: |
           cd tools/bench-graph-client
@@ -106,9 +104,3 @@ jobs:
                     --github-run-id "${{ github.run_id }}" \
                     --github-run-number "${{ github.run_number }}" \
                     --head-branch-name "${{ env.BRANCH_NAME }}"
-
-          ./main.py --benchmark-name "order_by" \
-                    --benchmark-results "../../tests/mgbench/order_by.json" \
-                    --github-run-id "${{ github.run_id }}" \
-                    --github-run-number "${{ github.run_number }}" \
-                    --head-branch-name "${{ env.BRANCH_NAME }}"

libs/setup.sh

@@ -259,7 +259,7 @@ repo_clone_try_double "${primary_urls[absl]}" "${secondary_urls[absl]}" "absl" "
 
 # jemalloc ea6b3e973b477b8061e0076bb257dbd7f3faa756
 JEMALLOC_COMMIT_VERSION="5.2.1"
-repo_clone_try_double "${primary_urls[jemalloc]}" "${secondary_urls[jemalloc]}" "jemalloc" "$JEMALLOC_COMMIT_VERSION"
+repo_clone_try_double "${secondary_urls[jemalloc]}" "${secondary_urls[jemalloc]}" "jemalloc" "$JEMALLOC_COMMIT_VERSION"
 
 # this is hack for cmake in libs to set path, and for FindJemalloc to use Jemalloc_INCLUDE_DIR
 pushd jemalloc

src/memory/global_memory_control.cpp

@@ -119,7 +119,7 @@ static bool my_commit(extent_hooks_t *extent_hooks, void *addr, size_t size, siz
 
   memgraph::utils::total_memory_tracker.Alloc(static_cast<int64_t>(length));
   if (GetQueriesMemoryControl().IsThreadTracked()) [[unlikely]] {
-    GetQueriesMemoryControl().TrackAllocOnCurrentThread(size);
+    GetQueriesMemoryControl().TrackFreeOnCurrentThread(size);
   }
 
   return false;

src/query/interpreter.cpp

@@ -238,16 +238,6 @@ std::optional<std::string> GetOptionalStringValue(query::Expression *expression,
   return {};
 };
 
-bool IsOrderByQuery(const std::vector<memgraph::query::Clause *> &clauses) {
-  for (const auto &clause : clauses) {
-    if (clause->GetTypeInfo() == Return::kType) {
-      auto *return_clause = utils::Downcast<Return>(clause);
-      return !return_clause->body_.order_by.empty();
-    }
-  }
-  return false;
-}
-
 bool IsAllShortestPathsQuery(const std::vector<memgraph::query::Clause *> &clauses) {
   for (const auto &clause : clauses) {
     if (clause->GetTypeInfo() != Match::kType) {
@@ -1601,8 +1591,8 @@ PreparedQuery PrepareCypherQuery(ParsedQuery parsed_query, std::map<std::string,
   }
 
   // If this is LOAD CSV query, use PoolResource without MonotonicMemoryResource as we want to reuse allocated memory
-  auto use_monotonic_memory = !contains_csv && !IsCallBatchedProcedureQuery(clauses) &&
-                              !IsAllShortestPathsQuery(clauses) && !IsOrderByQuery(clauses);
+  auto use_monotonic_memory =
+      !contains_csv && !IsCallBatchedProcedureQuery(clauses) && !IsAllShortestPathsQuery(clauses);
 
   MG_ASSERT(current_db.execution_db_accessor_, "Cypher query expects a current DB transaction");
   auto *dba =
@@ -1764,8 +1754,8 @@ PreparedQuery PrepareProfileQuery(ParsedQuery parsed_query, bool in_explicit_tra
 
   // If this is LOAD CSV, BatchedProcedure or AllShortest query, use PoolResource without MonotonicMemoryResource as we
   // want to reuse allocated memory
-  auto use_monotonic_memory = !contains_csv && !IsCallBatchedProcedureQuery(clauses) &&
-                              !IsAllShortestPathsQuery(clauses) && !IsOrderByQuery(clauses);
+  auto use_monotonic_memory =
+      !contains_csv && !IsCallBatchedProcedureQuery(clauses) && !IsAllShortestPathsQuery(clauses);
 
   MG_ASSERT(cypher_query, "Cypher grammar should not allow other queries in PROFILE");
   EvaluationContext evaluation_context;
@@ -3726,8 +3716,7 @@ Interpreter::PrepareResult Interpreter::Prepare(const std::string &query_string,
       auto const &clauses = cypher_query->single_query_->clauses_;
       bool hasAllShortestPaths = IsAllShortestPathsQuery(clauses);
       // Using PoolResource without MonotonicMemoryResouce for LOAD CSV reduces memory usage.
-      bool usePool = hasAllShortestPaths || IsCallBatchedProcedureQuery(clauses) || IsLoadCsvQuery(clauses) ||
-                     IsOrderByQuery(clauses);
+      bool usePool = hasAllShortestPaths || IsCallBatchedProcedureQuery(clauses) || IsLoadCsvQuery(clauses);
       return {usePool, hasAllShortestPaths};
     }();  // IILE
 

src/telemetry/telemetry.cpp

@@ -110,13 +110,7 @@ void Telemetry::CollectData(const std::string &event) {
   {
     std::lock_guard<std::mutex> guard(lock_);
     for (auto &collector : collectors_) {
-      try {
-        data[collector.first] = collector.second();
-      } catch (std::exception &e) {
-        spdlog::warn(fmt::format(
-            "Unknwon exception occured on in telemetry server {}, please contact support on https://memgr.ph/unknown ",
-            e.what()));
-      }
+      data[collector.first] = collector.second();
     }
   }
   if (event == "") {

src/utils/stat.hpp

@@ -25,32 +25,17 @@ namespace memgraph::utils {
 static constexpr int64_t VM_MAX_MAP_COUNT_DEFAULT{-1};
 
 /// Returns the number of bytes a directory is using on disk. If the given path
-/// isn't a directory, zero will be returned. If there are some files with
-/// wrong permission, it will be skipped
+/// isn't a directory, zero will be returned.
 template <bool IgnoreSymlink = true>
 inline uint64_t GetDirDiskUsage(const std::filesystem::path &path) {
   if (!std::filesystem::is_directory(path)) return 0;
 
-  if (!utils::HasReadAccess(path)) {
-    spdlog::warn(
-        "Skipping directory path on collecting directory disk usage '{}' because it is not readable, check file "
-        "ownership and read permissions!",
-        path);
-    return 0;
-  }
   uint64_t size = 0;
-  for (const auto &p : std::filesystem::directory_iterator(path)) {
+  for (auto &p : std::filesystem::directory_iterator(path)) {
     if (IgnoreSymlink && std::filesystem::is_symlink(p)) continue;
     if (std::filesystem::is_directory(p)) {
       size += GetDirDiskUsage(p);
     } else if (std::filesystem::is_regular_file(p)) {
-      if (!utils::HasReadAccess(p)) {
-        spdlog::warn(
-            "Skipping file path on collecting directory disk usage '{}' because it is not readable, check file "
-            "ownership and read permissions!",
-            p);
-        continue;
-      }
       size += std::filesystem::file_size(p);
     }
   }

tests/mgbench/README.md

@@ -96,7 +96,7 @@ Benchmarking different systems is challenging because the setup, environment, qu
 Listed below are the main scripts used to run the benchmarks:
 
 - `benchmark.py` - The main entry point used for starting and managing the execution of the benchmark. This script initializes all the necessary files, classes, and objects. It starts the database and the benchmark and gathers the results.
-- `base.py` - This is the base workload class. All other workloads are subclasses located in the workloads directory. For example, ldbc_interactive.py defines ldbc interactive dataset and queries (but this is NOT an official LDBC interactive workload). Each workload class can generate the dataset, use custom import ofthe dataset or provide a CYPHERL file for the import process..
+- `base.py` - This is the base workload class. All other workloads are subclasses located in the workloads directory. For example, ldbc_interactive.py defines ldbc interactive dataset and queries (but this is NOT an official LDBC interactive workload). Each workload class can generate the dataset, use custom import of the dataset or provide a CYPHERL file for the import process..
 - `runners.py` - The script that configures, starts, and stops the database.
 - `client.cpp` - Client for querying the database.
 - `graph_bench.py` - Script that starts all tests from Benchgraph.

tests/mgbench/benchmark.py

@@ -239,7 +239,11 @@ def sanitize_args(args):
     assert args.benchmarks != None, helpers.list_available_workloads()
     assert args.num_workers_for_import > 0
     assert args.num_workers_for_benchmark > 0
-    assert args.export_results != None, "Pass where will results be saved"
+    assert (
+        args.export_results != None
+        or args.export_results_on_disk_txn != None
+        or args.export_results_in_memory_analytical != None
+    ), "Pass where will results be saved"
     assert args.single_threaded_runtime_sec >= 1, "Low runtime value, consider extending time for more accurate results"
     assert (
         args.workload_realistic == None or args.workload_mixed == None
@@ -686,25 +690,25 @@ def run_target_workload(benchmark_context, workload, bench_queries, vendor_runne
             run_isolated_workload_with_authorization(vendor_runner, client, bench_queries, group, workload, results)
 
 
-# TODO: (andi) Reorder functions in top-down notion in order to improve readibility
+# TODO: (andi) Reorder functions in top-down notion in order to improve readibility -> does this referes to run_target_workloads function or the code around?
 def run_target_workloads(benchmark_context, target_workloads, bench_results):
     for workload, bench_queries in target_workloads:
         log.info(f"Started running {str(workload.NAME)} workload")
-
         benchmark_context.set_active_workload(workload.NAME)
         benchmark_context.set_active_variant(workload.get_variant())
 
+        # TODO(gitbuda): What's the semantic of --export-results-xyz flags? NOTE: avoid nested if/else statements
         if workload.is_disk_workload() and benchmark_context.export_results_on_disk_txn:
             run_on_disk_transactional_benchmark(benchmark_context, workload, bench_queries, bench_results.disk_results)
-        else:
-            run_in_memory_transactional_benchmark(
-                benchmark_context, workload, bench_queries, bench_results.in_memory_txn_results
+            return
+        if benchmark_context.export_results_in_memory_analytical:
+            run_in_memory_analytical_benchmark(
+                benchmark_context, workload, bench_queries, bench_results.in_memory_analytical_results
             )
-
-            if benchmark_context.export_results_in_memory_analytical:
-                run_in_memory_analytical_benchmark(
-                    benchmark_context, workload, bench_queries, bench_results.in_memory_analytical_results
-                )
+            return
+        run_in_memory_transactional_benchmark(
+            benchmark_context, workload, bench_queries, bench_results.in_memory_txn_results
+        )
 
 
 def run_on_disk_transactional_benchmark(benchmark_context, workload, bench_queries, disk_results):

tests/mgbench/runners.py

@@ -811,7 +811,7 @@ class MemgraphDocker(BaseRunner):
                 "-it",
                 "-p",
                 self._bolt_port + ":" + self._bolt_port,
-                "memgraph/memgraph:2.7.0",
+                "memgraph/memgraph:2.14.0",  # TODO(gitbuda): parametrize & fallback to the latest version.
                 "--storage_wal_enabled=false",
                 "--storage_recover_on_startup=true",
                 "--storage_snapshot_interval_sec",

tests/mgbench/workloads/order_by.py

@@ -1,46 +0,0 @@
-# Copyright 2023 Memgraph Ltd.
-#
-# Use of this software is governed by the Business Source License
-# included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
-# License, and you may not use this file except in compliance with the Business Source License.
-#
-# As of the Change Date specified in that file, in accordance with
-# the Business Source License, use of this software will be governed
-# by the Apache License, Version 2.0, included in the file
-# licenses/APL.txt.
-
-from workloads.base import Workload
-
-
-class OrderBy(Workload):
-    NAME = "order_by"
-    CARDINALITY = 10000
-
-    def indexes_generator(self):
-        return [
-            ("CREATE INDEX ON :Node;", {}),
-            ("CREATE INDEX ON :Node(prop1);", {}),
-            ("CREATE INDEX ON :Node(prop2);", {}),
-            ("CREATE INDEX ON :Node(prop3);", {}),
-        ]
-
-    def dataset_generator(self):
-        queries = []
-        for i in range(0, OrderBy.CARDINALITY):
-            queries.append(
-                (
-                    """CREATE
-(:Node {prop1: $id, prop2: $id, prop3: $id
-});
-""",
-                    {"id": i},
-                )
-            )
-
-        return queries
-
-    def benchmark__test__order_by(self):
-        return (
-            "MATCH (n:Node) RETURN n ORDER BY n.prop1",
-            {},
-        )