From 1a20c557b8b8e9f04b6d3636c9495a343aff3674 Mon Sep 17 00:00:00 2001 From: Teon Banek Date: Thu, 1 Aug 2019 13:13:38 +0200 Subject: [PATCH] Update storage API docs with thrown exceptions Summary: The documentation includes `std` exceptions like `std::bad_alloc` or `std::system_error`, for which there's probably nothing we can do. This may seem unnecessary, but it will be really helpful when writing the C API for interfacing with custom modules and plugins, as well as when switching to storage v2 API. In general, we should start updating the documentation of functions which may throw exceptions. This ought to be enforced in code review, so that the implementation and documentation are kept in sync. Reviewers: mferencevic, mtomic, msantl Reviewed By: mferencevic Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D2288 --- .../single_node/graph_db_accessor.hpp | 38 ++++++++++++++++--- src/storage/common/locking/record_lock.hpp | 1 + src/storage/single_node/mvcc/version_list.hpp | 12 +++++- src/storage/single_node/record_accessor.hpp | 24 ++++++++++-- src/storage/single_node/vertex_accessor.hpp | 29 +++++++++++--- src/transactions/lock_store.hpp | 2 + src/transactions/single_node/engine.hpp | 3 ++ src/transactions/transaction.hpp | 4 +- 8 files changed, 96 insertions(+), 17 deletions(-) diff --git a/src/database/single_node/graph_db_accessor.hpp b/src/database/single_node/graph_db_accessor.hpp index 85f0ec5e4..a4c3a9122 100644 --- a/src/database/single_node/graph_db_accessor.hpp +++ b/src/database/single_node/graph_db_accessor.hpp @@ -91,6 +91,8 @@ class GraphDbAccessor { * @param check_empty If the vertex should be checked for existing edges * before deletion. * @return If or not the vertex was deleted. + * @throw utils::LockTimeoutException + * @throw SerializationError */ bool RemoveVertex(VertexAccessor &vertex_accessor, bool check_empty = true); @@ -99,6 +101,9 @@ class GraphDbAccessor { * and incoming connections. * * @param vertex_accessor Accessor to a vertex. + * @throw RecordDeletedError + * @throw utils::LockTimeoutException + * @throw SerializationError */ void DetachRemoveVertex(VertexAccessor &vertex_accessor); @@ -294,6 +299,9 @@ class GraphDbAccessor { * recovering from durability. * * @return An accessor to the edge. + * @throw RecordDeletedError + * @throw utils::LockTimeoutException + * @throw SerializationError */ EdgeAccessor InsertEdge(VertexAccessor &from, VertexAccessor &to, storage::EdgeType type, @@ -310,6 +318,9 @@ class GraphDbAccessor { * side. * @param remove_in_edge If the edge should be removed from the its * destination side. + * @throw RecordDeletedError + * @throw utils::LockTimeoutException + * @throw SerializationError */ void RemoveEdge(EdgeAccessor &edge, bool remove_out_edge = true, bool remove_in_edge = true); @@ -413,13 +424,14 @@ class GraphDbAccessor { * * @param label - label to build for * @param property - property to build for + * @throw IndexExistsException + * @throw TransactionException */ void BuildIndex(storage::Label label, storage::Property property); /// Deletes the index responisble for (label, property). /// - /// @throws IndexTransactionException if it can't obtain a blocking - /// transaction. + /// @throw TransactionException if it can't obtain a blocking transaction. void DeleteIndex(storage::Label label, storage::Property property); /// Populates index with vertices containing the key @@ -433,10 +445,10 @@ class GraphDbAccessor { * properties. * If the constraint already exists, this method does nothing. * - * @throws ConstraintViolationException if constraint couldn't be build + * @throw ConstraintViolationException if constraint couldn't be build * due to existing constraint violation. - * @throws TransactionEngineError if the engine doesn't accept transactions. - * @throws mvcc::SerializationError on serialization errors. + * @throw TransactionEngineError if the engine doesn't accept transactions. + * @throw mvcc::SerializationError on serialization errors. */ void BuildUniqueConstraint(storage::Label label, const std::vector &properties); @@ -444,6 +456,7 @@ class GraphDbAccessor { /** * Deletes existing unique constraint. * If the constraint doesn't exist, this method does nothing. + * @throw TransactionException */ void DeleteUniqueConstraint(storage::Label label, const std::vector &properties); @@ -574,7 +587,10 @@ class GraphDbAccessor { /** Returns the id of this accessor's transaction */ tx::TransactionId transaction_id() const; - /** Advances transaction's command id by 1. */ + /** + * Advances transaction's command id by 1. + * @throw TransactionException + */ void AdvanceCommand(); /** Commit transaction. */ @@ -622,6 +638,9 @@ class GraphDbAccessor { * @param label - label that was added * @param vertex_accessor - vertex_accessor that was updated * @param vertex - vertex that was updated + * @throw utils::LockTimeoutException + * @throw SerializationError + * @throw ConstraintViolationException */ void UpdateOnAddLabel(storage::Label label, const VertexAccessor &vertex_accessor, @@ -632,6 +651,8 @@ class GraphDbAccessor { * * @param label - label that was removed * @param vertex_accessor - vertex_accessor that was updated + * @throw utils::LockTimeoutException + * @throw SerializationError */ void UpdateOnRemoveLabel(storage::Label label, const RecordAccessor &accessor); @@ -643,6 +664,8 @@ class GraphDbAccessor { * @param previous_value - previous value of the property * @param vertex_accessor - vertex_accessor that was updated * @param vertex - vertex that was updated + * @throw utils::LockTimeoutException + * @throw SerializationError */ void UpdateOnRemoveProperty(storage::Property property, const PropertyValue &previous_value, @@ -657,6 +680,9 @@ class GraphDbAccessor { * @param new_value - new value of the property * @param vertex_accessor - vertex accessor that was updated * @param vertex - vertex that was updated + * @throw utils::LockTimeoutException + * @throw SerializationError + * @throw ConstraintViolationException */ void UpdateOnAddProperty(storage::Property property, const PropertyValue &previous_value, diff --git a/src/storage/common/locking/record_lock.hpp b/src/storage/common/locking/record_lock.hpp index 78aaf7801..a814c4a34 100644 --- a/src/storage/common/locking/record_lock.hpp +++ b/src/storage/common/locking/record_lock.hpp @@ -14,6 +14,7 @@ class Transaction; class RecordLock { public: + /// @throw utils::LockTimeoutException LockStatus Lock(const tx::Transaction &id, tx::Engine &engine); void Unlock(); diff --git a/src/storage/single_node/mvcc/version_list.hpp b/src/storage/single_node/mvcc/version_list.hpp index a4dd27c03..30b9aefc4 100644 --- a/src/storage/single_node/mvcc/version_list.hpp +++ b/src/storage/single_node/mvcc/version_list.hpp @@ -191,6 +191,8 @@ class VersionList { * older visible record when this update is called. * * @param t The transaction + * @throw utils::LockTimeoutException + * @throw SerializationError */ T *update(const tx::Transaction &t) { DCHECK(head_ != nullptr) << "Head is nullptr on update."; @@ -208,7 +210,11 @@ class VersionList { return update(old_record, t); } - /** Makes the given record as being expired by the given transaction. */ + /** + * Makes the given record as being expired by the given transaction. + * @throw utils::LockTimeoutException + * @throw SerializationError + */ void remove(T *record, const tx::Transaction &t) { DCHECK(record != nullptr) << "Record is nullptr on removal."; lock_and_validate(record, t); @@ -220,6 +226,8 @@ class VersionList { int64_t cypher_id() { return utils::MemcpyCast(gid_); } private: + /// @throw utils::LockTimeoutException + /// @throw SerializationError void lock_and_validate(T *record, const tx::Transaction &t) { DCHECK(record != nullptr) << "Record is nullptr on lock and validation."; @@ -234,6 +242,8 @@ class VersionList { throw SerializationError(); } + /// @throw utils::LockTimeoutException + /// @throw SerializationError T *update(T *record, const tx::Transaction &t) { DCHECK(record != nullptr) << "Record is nullptr on update."; lock_and_validate(record, t); diff --git a/src/storage/single_node/record_accessor.hpp b/src/storage/single_node/record_accessor.hpp index 1f025a16a..79ca81ebc 100644 --- a/src/storage/single_node/record_accessor.hpp +++ b/src/storage/single_node/record_accessor.hpp @@ -54,13 +54,27 @@ class RecordAccessor { /** Gets the property for the given key. */ PropertyValue PropsAt(storage::Property key) const; - /** Sets a value on the record for the given property. */ + /** + * Sets a value on the record for the given property. + * @throw RecordDeletedError + * @throw utils::LockTimeoutException + * @throw SerializationError + * @throw ConstraintViolationException + */ void PropsSet(storage::Property key, PropertyValue value); - /** Erases the property for the given key. */ + /** + * Erases the property for the given key. + * @throw utils::LockTimeoutException + * @throw SerializationError + */ void PropsErase(storage::Property key); - /** Removes all the properties from this record. */ + /** + * Removes all the properties from this record. + * @throw utils::LockTimeoutException + * @throw SerializationError + */ void PropsClear(); /** Returns the properties of this record. */ @@ -127,7 +141,9 @@ class RecordAccessor { * It is not legal to call this function on a Vertex/Edge that has been * deleted in the current transaction+command. * - * @throws RecordDeletedError + * @throw RecordDeletedError + * @throw utils::LockTimeoutException + * @throw SerializationError */ TRecord &update() const; diff --git a/src/storage/single_node/vertex_accessor.hpp b/src/storage/single_node/vertex_accessor.hpp index 3cdfa6202..1297c5a76 100644 --- a/src/storage/single_node/vertex_accessor.hpp +++ b/src/storage/single_node/vertex_accessor.hpp @@ -58,11 +58,22 @@ class VertexAccessor final : public RecordAccessor { /** Returns the number of incoming edges. */ size_t in_degree() const; - /** Adds a label to the Vertex. If the Vertex already has that label the call - * has no effect. */ + /** + * Add a label to the Vertex. + * If the Vertex already has that label the call has no effect. + * @throw RecordDeletedError + * @throw utils::LockTimeoutException + * @throw SerializationError + * @throw ConstraintViolationException + */ void add_label(storage::Label label); - /** Removes a label from the Vertex. */ + /** + * Removes a label from the Vertex. + * @throw RecordDeletedError + * @throw utils::LockTimeoutException + * @throw SerializationError + */ void remove_label(storage::Label label); /** Indicates if the Vertex has the given label. */ @@ -138,12 +149,20 @@ class VertexAccessor final : public RecordAccessor { /** Removes the given edge from the outgoing edges of this vertex. Note that * this operation should always be accompanied by the removal of the edge from - * the incoming edges on the other side and edge deletion. */ + * the incoming edges on the other side and edge deletion. + * @throw RecordDeletedError + * @throw utils::LockTimeoutException + * @throw SerializationError + */ void RemoveOutEdge(mvcc::VersionList *edge); /** Removes the given edge from the incoming edges of this vertex. Note that * this operation should always be accompanied by the removal of the edge from - * the outgoing edges on the other side and edge deletion. */ + * the outgoing edges on the other side and edge deletion. + * @throw RecordDeletedError + * @throw utils::LockTimeoutException + * @throw SerializationError + */ void RemoveInEdge(mvcc::VersionList *edge); }; diff --git a/src/transactions/lock_store.hpp b/src/transactions/lock_store.hpp index d0cca72c3..fd5a92001 100644 --- a/src/transactions/lock_store.hpp +++ b/src/transactions/lock_store.hpp @@ -20,6 +20,7 @@ class LockStore { public: LockHolder() = default; + /// @throw utils::LockTimeoutException LockHolder(RecordLock *lock, const Transaction &tx, tx::Engine &engine) : lock_(lock) { DCHECK(lock != nullptr) << "Lock is nullptr."; @@ -57,6 +58,7 @@ class LockStore { }; public: + /// @throw utils::LockTimeoutException void Take(RecordLock *lock, const tx::Transaction &tx, tx::Engine &engine) { // Creating a lock holder locks the version list to the given transaction. // Note that it's an op that can take a long time (if there are multiple diff --git a/src/transactions/single_node/engine.hpp b/src/transactions/single_node/engine.hpp index 0d9a30252..278bca128 100644 --- a/src/transactions/single_node/engine.hpp +++ b/src/transactions/single_node/engine.hpp @@ -29,12 +29,15 @@ class Engine final { Engine &operator=(const Engine &) = delete; Engine &operator=(Engine &&) = delete; + /// @throw TransactionEngineError Transaction *Begin(); /// Blocking transactions are used when we can't allow any other transaction to /// run (besides this one). This is the reason why this transactions blocks the /// engine from creating new transactions and waits for the existing ones to /// finish. + /// @throw TransactionEngineError Transaction *BeginBlocking(std::optional parent_tx); + /// @throw TransactionException CommandId Advance(TransactionId id); CommandId UpdateCommand(TransactionId id); void Commit(const Transaction &t); diff --git a/src/transactions/transaction.hpp b/src/transactions/transaction.hpp index 43e6c2486..51bdb4d79 100644 --- a/src/transactions/transaction.hpp +++ b/src/transactions/transaction.hpp @@ -54,6 +54,7 @@ class Transaction final { public: /// Acquires the lock over the given RecordLock, preventing other transactions /// from doing the same + /// @throw utils::LockTimeoutException void TakeLock(RecordLock &lock) const { locks_.Take(&lock, *this, engine_); } /// Transaction's id. Unique in the engine that owns it @@ -81,7 +82,8 @@ class Transaction final { auto blocking() const { return blocking_; } private: - // Function used to advance the command. + /// Function used to advance the command. + /// @throw TransactionError CommandId AdvanceCommand() { if (cid_ == std::numeric_limits::max()) { throw TransactionError(