Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ build

# IDE-related directories and files
.idea
cmake-build-debug
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@
[submodule "submodules/Catch2"]
path = submodules/Catch2
url = https://github.com/catchorg/Catch2.git
[submodule "submodules/abseil-cpp"]
path = submodules/abseil-cpp
url = https://github.com/abseil/abseil-cpp.git
Comment on lines +7 to +9

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Submodule needs initialization

The submodule directory is empty, indicating that it hasn't been initialized. Please run:

git submodule update --init --recursive
🔗 Analysis chain

LGTM! Verify submodule initialization.

The addition of abseil-cpp as a submodule follows the project's conventions.

Let's verify the submodule initialization:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the abseil-cpp submodule is properly initialized

# Test: Check if the submodule directory exists and is not empty
if [ -d "submodules/abseil-cpp" ] && [ "$(ls -A submodules/abseil-cpp)" ]; then
    echo "Submodule is properly initialized"
else
    echo "Warning: Submodule directory is missing or empty"
fi

# Test: Verify the submodule commit
git submodule status submodules/abseil-cpp

Length of output: 294

19 changes: 17 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,17 @@ else()
endif()
message(STATUS "Building using ${SPIDER_LIBS_STRING} libraries")

# Find and setup Boost Library
if(SPIDER_USE_STATIC_LIBS)
set(Boost_USE_STATIC_LIBS ON)
endif()
find_package(Boost 1.74 REQUIRED)
if(Boost_FOUND)
message(STATUS "Found Boost ${Boost_VERSION}")
else()
message(FATAL_ERROR "Could not find ${SPIDER_LIBS_STRING} libraries for Boost")
endif()

# Find and setup fmt
find_package(fmt 8.0.1 REQUIRED)
if(fmt_FOUND)
Expand Down Expand Up @@ -106,17 +117,21 @@ else()
endif()

# Find and setup MariaDBClient library
if(CLP_USE_STATIC_LIBS)
if(SPIDER_USE_STATIC_LIBS)
# NOTE: We can't statically link to MariaDBClient since it's GPL
message(AUTHOR_WARNING "MariaDBClient cannot be statically linked due to its license.")
endif()
find_package(MariaDBClient 3.1.0 REQUIRED)
if(MariaDBClient_FOUND)
message(STATUS "Found MariaDBClient ${MariaDBClient_VERSION}")
else()
message(FATAL_ERROR "Could not find ${CLP_LIBS_STRING} libraries for MariaDBClient")
message(FATAL_ERROR "Could not find ${SPIDER_LIBS_STRING} libraries for MariaDBClient")
endif()

# Add abseil-cpp
set(ABSL_PROPAGATE_CXX_STD ON)
add_subdirectory(submodules/abseil-cpp EXCLUDE_FROM_ALL)

find_package(Threads REQUIRED)

add_subdirectory(src/spider)
Expand Down
19 changes: 17 additions & 2 deletions src/spider/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
# set variable as CACHE INTERNAL to access it from other scope
set(SPIDER_CORE_SOURCES storage/MetadataStorage.hpp CACHE INTERNAL "spider core source files")

set(SPIDER_CORE_SOURCES
core/Error.hpp
core/Data.hpp
core/Task.hpp
core/TaskGraph.hpp
storage/MetadataStorage.hpp
storage/DataStorage.hpp
storage/MysqlStorage.cpp
storage/MysqlStorage.hpp
CACHE INTERNAL
"spider core source files"
)
set(SPIDER_WORKER_SOURCES worker/worker.cpp CACHE INTERNAL "spider worker source files")

add_executable(spider_worker)
Expand All @@ -11,4 +21,9 @@ target_sources(
${SPIDER_CORE_SOURCES}
${SPIDER_WORKER_SOURCES}
)
target_link_libraries(
spider_worker
Boost::boost
absl::flat_hash_map
)
add_executable(spider::worker ALIAS spider_worker)
35 changes: 35 additions & 0 deletions src/spider/core/Data.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#ifndef SPIDER_CORE_DATA_HPP
#define SPIDER_CORE_DATA_HPP

#include <boost/uuid/random_generator.hpp>
#include <boost/uuid/uuid.hpp>
#include <optional>
#include <string>
#include <utility>

class Data {
private:
boost::uuids::uuid m_id;
std::optional<std::string> m_key;
std::string m_value;

Comment on lines +10 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance const correctness and exception specifications

The class design is good with proper encapsulation. Consider enhancing it with const correctness and explicit exception specifications.

-    auto get_id() -> boost::uuids::uuid { return m_id; }
+    auto get_id() const noexcept -> boost::uuids::uuid { return m_id; }

-    auto get_key() -> std::optional<std::string> { return m_key; }
+    auto get_key() const noexcept -> std::optional<std::string> { return m_key; }

-    auto get_value() -> std::string { return m_value; }
+    auto get_value() const noexcept -> std::string { return m_value; }

Committable suggestion was skipped due to low confidence.

void init_id() {
boost::uuids::random_generator gen;
m_id = gen();
}

public:
explicit Data(std::string value) : m_value(std::move(value)) { init_id(); }

Data(std::string key, std::string value) : m_key(std::move(key)), m_value(std::move(value)) {
init_id();
}

[[nodiscard]] auto get_id() const -> boost::uuids::uuid { return m_id; }

[[nodiscard]] auto get_key() const -> std::optional<std::string> { return m_key; }

[[nodiscard]] auto get_value() const -> std::string { return m_value; }
};

#endif // SPIDER_CORE_DATA_HPP
31 changes: 31 additions & 0 deletions src/spider/core/Error.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#ifndef SPIDER_CORE_ERROR_HPP
#define SPIDER_CORE_ERROR_HPP

#include <cstdint>
#include <string>
#include <utility>

namespace spider::core {
enum class StorageErrType : std::uint8_t {
Success = 0,
ConnectionErr,
DbNotFound,
KeyNotFoundErr,
DuplicateKeyErr,
ConstraintViolationErr
};

struct StorageErr {
StorageErrType type;
std::string description;

StorageErr() : type(StorageErrType::Success) {}

StorageErr(StorageErrType type, std::string description)
: type(type),
description(std::move(description)) {}
};

} // namespace spider::core

#endif // SPIDER_CORE_ERROR_HPP
135 changes: 135 additions & 0 deletions src/spider/core/Task.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
#ifndef SPIDER_CORE_TASK_HPP
#define SPIDER_CORE_TASK_HPP

#include <boost/uuid/random_generator.hpp>
#include <boost/uuid/uuid.hpp>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Include the appropriate header for UUID generator

To use boost::uuids::random_generator_mt19937, include the header <boost/uuid/uuid_generators.hpp>.

Apply this diff:

 #include <boost/uuid/uuid.hpp>
+#include <boost/uuid/uuid_generators.hpp>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#include <boost/uuid/uuid.hpp>
#include <boost/uuid/uuid.hpp>
#include <boost/uuid/uuid_generators.hpp>

#include <cstddef>
#include <cstdint>
#include <optional>
#include <string>
#include <tuple>
#include <utility>
#include <vector>

namespace spider::core {

class TaskInput {
private:
std::optional<std::tuple<boost::uuids::uuid, std::uint8_t>> m_task_output;
std::optional<std::string> m_value;
std::optional<boost::uuids::uuid> m_data_id;
std::string m_type;

public:
TaskInput(boost::uuids::uuid output_task_id, std::uint8_t position, std::string type)
: m_task_output({output_task_id, position}),
m_type(std::move(type)) {};
TaskInput(std::string value, std::string type)
: m_value(std::move(value)),
m_type(std::move(type)) {};
TaskInput(boost::uuids::uuid data_id, std::string type)
: m_data_id(data_id),
m_type(std::move(type)) {};

[[nodiscard]] auto get_task_output(
) const -> std::optional<std::tuple<boost::uuids::uuid, std::uint8_t>> {
return m_task_output;
}

[[nodiscard]] auto get_value() const -> std::optional<std::string> { return m_value; }

[[nodiscard]] auto get_data_id() const -> std::optional<boost::uuids::uuid> {
return m_data_id;
}

[[nodiscard]] auto get_type() const -> std::string { return m_type; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Return const reference from getter to avoid unnecessary copying

The get_type() function returns a std::string by value, which may cause unnecessary copying. Consider returning a const std::string& to improve performance.

Apply this diff:

-        [[nodiscard]] auto get_type() const -> std::string { return m_type; }
+        [[nodiscard]] auto get_type() const -> const std::string& { return m_type; }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[[nodiscard]] auto get_type() const -> std::string { return m_type; }
[[nodiscard]] auto get_type() const -> const std::string& { return m_type; }

};

class TaskOutput {
private:
std::optional<std::string> m_value;
std::optional<boost::uuids::uuid> m_data_id;
std::string m_type;

public:
TaskOutput(std::string value, std::string type)
: m_value(std::move(value)),
m_type(std::move(type)) {}

TaskOutput(boost::uuids::uuid data_id, std::string type)
: m_data_id(data_id),
m_type(std::move(type)) {}

[[nodiscard]] auto get_value() const -> std::optional<std::string> { return m_value; }

[[nodiscard]] auto get_data_id() const -> std::optional<boost::uuids::uuid> {
return m_data_id;
}

[[nodiscard]] auto get_type() const -> std::string { return m_type; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Return const reference from getter to avoid unnecessary copying

The get_type() function returns a std::string by value, which may cause unnecessary copying. Consider returning a const std::string& to improve performance.

Apply this diff:

-        [[nodiscard]] auto get_type() const -> std::string { return m_type; }
+        [[nodiscard]] auto get_type() const -> const std::string& { return m_type; }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[[nodiscard]] auto get_type() const -> std::string { return m_type; }
[[nodiscard]] auto get_type() const -> const std::string& { return m_type; }

};

class TaskInstance {};

enum class TaskState : std::uint8_t {
Pending,
Ready,
Running,
Succeed,
Failed,
Canceled,
};

enum class TaskCreatorType : std::uint8_t {
Client = 0,
Task,
};

class Task {
private:
boost::uuids::uuid m_id;
std::string m_function_name;
TaskState m_state = TaskState::Pending;
TaskCreatorType m_creator_type;
boost::uuids::uuid m_creator_id;
float m_timeout = 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using a more precise time representation for 'm_timeout'

Using float for m_timeout may lead to precision issues and does not convey time semantics. Consider using a more precise and expressive type such as std::chrono::duration to represent time intervals.

std::vector<TaskInput> m_inputs;
std::vector<TaskOutput> m_outputs;
Comment on lines +96 to +97

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider pre-allocating vectors if size is known

If the number of inputs/outputs is known at construction time, consider using reserve() to prevent reallocation.

Example:

// In constructor
m_inputs.reserve(expected_inputs);
m_outputs.reserve(expected_outputs);


public:
Task(std::string function_name, TaskCreatorType creator_type, boost::uuids::uuid creator_id)
: m_function_name(std::move(function_name)),
m_creator_type(creator_type),
m_creator_id(creator_id) {
boost::uuids::random_generator gen;
m_id = gen();
Comment on lines +104 to +105

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use a thread-safe UUID generator

boost::uuids::random_generator may not be thread-safe. Consider using boost::uuids::random_generator_mt19937, which is designed for thread-safe UUID generation.

Apply this diff:

-            boost::uuids::random_generator gen;
+            boost::uuids::random_generator_mt19937 gen;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
boost::uuids::random_generator gen;
m_id = gen();
boost::uuids::random_generator_mt19937 gen;
m_id = gen();

}

void add_input(TaskInput const& input) { m_inputs.emplace_back(input); }

void add_output(TaskOutput const& output) { m_outputs.emplace_back(output); }

[[nodiscard]] auto get_id() const -> boost::uuids::uuid { return m_id; }

[[nodiscard]] auto get_function_name() const -> std::string { return m_function_name; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Return const reference from getter to avoid unnecessary copying

The get_function_name() function returns a std::string by value, which may cause unnecessary copying. Consider returning a const std::string& to improve performance.

Apply this diff:

-        [[nodiscard]] auto get_function_name() const -> std::string { return m_function_name; }
+        [[nodiscard]] auto get_function_name() const -> const std::string& { return m_function_name; }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[[nodiscard]] auto get_function_name() const -> std::string { return m_function_name; }
[[nodiscard]] auto get_function_name() const -> const std::string& { return m_function_name; }


[[nodiscard]] auto get_state() const -> TaskState { return m_state; }

[[nodiscard]] auto get_creator_type() const -> TaskCreatorType { return m_creator_type; }

[[nodiscard]] auto get_creator_id() const -> boost::uuids::uuid { return m_creator_id; }

[[nodiscard]] auto get_timeout() const -> float { return m_timeout; }

[[nodiscard]] auto get_num_inputs() const -> size_t { return m_inputs.size(); }

[[nodiscard]] auto get_num_outputs() const -> size_t { return m_outputs.size(); }

[[nodiscard]] auto get_input(uint64_t index) const -> TaskInput { return m_inputs[index]; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Return const reference from getter to avoid unnecessary copying

The get_input() function returns a TaskInput by value, which may result in unnecessary copying. Consider returning a const TaskInput& instead.

Apply this diff:

-        [[nodiscard]] auto get_input(uint64_t index) const -> TaskInput { return m_inputs[index]; }
+        [[nodiscard]] auto get_input(uint64_t index) const -> const TaskInput& { return m_inputs[index]; }

Similarly for get_output():

-        [[nodiscard]] auto get_output(uint64_t index) const -> TaskOutput { return m_outputs[index]; }
+        [[nodiscard]] auto get_output(uint64_t index) const -> const TaskOutput& { return m_outputs[index]; }

Committable suggestion was skipped due to low confidence.


[[nodiscard]] auto get_output(uint64_t index) const -> TaskOutput { return m_outputs[index]; }
Comment on lines +128 to +130

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add bounds checking for vector access

The get_input and get_output methods should validate the index before accessing the vectors to prevent out-of-bounds access.

Apply this diff:

-    [[nodiscard]] auto get_input(uint64_t index) const -> TaskInput { return m_inputs[index]; }
+    [[nodiscard]] auto get_input(uint64_t index) const -> TaskInput {
+        if (index >= m_inputs.size()) {
+            throw std::out_of_range("Input index out of bounds");
+        }
+        return m_inputs[index];
+    }

-    [[nodiscard]] auto get_output(uint64_t index) const -> TaskOutput { return m_outputs[index]; }
+    [[nodiscard]] auto get_output(uint64_t index) const -> TaskOutput {
+        if (index >= m_outputs.size()) {
+            throw std::out_of_range("Output index out of bounds");
+        }
+        return m_outputs[index];
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[[nodiscard]] auto get_input(uint64_t index) const -> TaskInput { return m_inputs[index]; }
[[nodiscard]] auto get_output(uint64_t index) const -> TaskOutput { return m_outputs[index]; }
[[nodiscard]] auto get_input(uint64_t index) const -> TaskInput {
if (index >= m_inputs.size()) {
throw std::out_of_range("Input index out of bounds");
}
return m_inputs[index];
}
[[nodiscard]] auto get_output(uint64_t index) const -> TaskOutput {
if (index >= m_outputs.size()) {
throw std::out_of_range("Output index out of bounds");
}
return m_outputs[index];
}

};

} // namespace spider::core

#endif // SPIDER_CORE_TASK_HPP
79 changes: 79 additions & 0 deletions src/spider/core/TaskGraph.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#ifndef SPIDER_CORE_TASKGRAPH_HPP
#define SPIDER_CORE_TASKGRAPH_HPP

#include <absl/container/flat_hash_map.h>

#include <boost/uuid/uuid.hpp>
#include <optional>
#include <utility>
#include <vector>

#include "Task.hpp"

namespace spider::core {

class TaskGraph {
private:
absl::flat_hash_map<boost::uuids::uuid, Task> m_tasks;
std::vector<std::pair<boost::uuids::uuid, boost::uuids::uuid>> m_dependencies;

public:
auto add_child_task(Task const& task, std::vector<boost::uuids::uuid> const& parents) -> bool {
boost::uuids::uuid const task_id = task.get_id();
for (boost::uuids::uuid const parent_id : parents) {
if (!m_tasks.contains(parent_id)) {
return false;
}
}
if (m_tasks.contains(task.get_id())) {
return false;
}

m_tasks.emplace(task_id, task);
for (boost::uuids::uuid const parent_id : parents) {
m_dependencies.emplace_back(parent_id, task_id);
}
return true;
}

[[nodiscard]] auto get_task(boost::uuids::uuid id) const -> std::optional<Task> {
if (m_tasks.contains(id)) {
return m_tasks.at(id);
}
return std::nullopt;
}

[[nodiscard]] auto get_child_tasks(boost::uuids::uuid id
) const -> std::vector<boost::uuids::uuid> {
std::vector<boost::uuids::uuid> children;
for (std::pair<boost::uuids::uuid, boost::uuids::uuid> const dep : m_dependencies) {
if (dep.first == id) {
children.emplace_back(dep.second);
}
}
return children;
}

[[nodiscard]] auto get_parent_tasks(boost::uuids::uuid id
) const -> std::vector<boost::uuids::uuid> {
std::vector<boost::uuids::uuid> parents;
for (std::pair<boost::uuids::uuid, boost::uuids::uuid> const dep : m_dependencies) {
if (dep.second == id) {
parents.emplace_back(dep.first);
}
}
return parents;
}

[[nodiscard]] auto get_tasks() const -> absl::flat_hash_map<boost::uuids::uuid, Task> const& {
return m_tasks;
}

[[nodiscard]] auto get_dependencies(
) const -> std::vector<std::pair<boost::uuids::uuid, boost::uuids::uuid>> const& {
return m_dependencies;
}
};
} // namespace spider::core

#endif // SPIDER_CORE_TASKGRAPH_HPP
Loading