Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug fix/non recursive plan handling #14475

Merged
merged 9 commits into from
Jul 8, 2021
Merged

Bug fix/non recursive plan handling #14475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f5b97e9
Make ExecutionNode::walk and walkSubqueriesFirst non-recursive.
mpoeter Jul 7, 2021
af8977d
Make cost estimation and invalidation non-recursive.
mpoeter Jul 7, 2021
a5a2322
Use HashSet::find/contains instead of std::find.
mpoeter Jul 7, 2021
c662b91
Refactor serialization of execution nodes and make it non-recursive.
mpoeter Jul 7, 2021
a2a8c77
Update CHANGELOG
mpoeter Jul 7, 2021
1f89016
Adapt test do use larger number of nodes.
mpoeter Jul 7, 2021
e758a35
Apply suggestions from code review
mpoeter Jul 7, 2021
013fdb2
Adapt unit tests.
mpoeter Jul 7, 2021
1598566
Address review comments.
mpoeter Jul 8, 2021
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
3 changes: 3 additions & 0 deletions CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
devel
-----
* Adapt various places related to handling of execution plans non-recursive
in order to avoid stack overflows. This allows us now to execute much larger
queries.

* Updated ArangoDB Starter to 0.15.1-preview-3.

Expand Down
57 changes: 13 additions & 44 deletions arangod/Aql/ClusterNodes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,18 +153,14 @@ std::unique_ptr<ExecutionBlock> RemoteNode::createBlock(
queryId());
}

/// @brief toVelocyPack, for RemoteNode
void RemoteNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const {
/// @brief doToVelocyPack, for RemoteNode
void RemoteNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
// call base class method
DistributeConsumerNode::toVelocyPackHelperInternal(nodes, flags, seen);
DistributeConsumerNode::doToVelocyPack(nodes, flags);

nodes.add("database", VPackValue(_vocbase->name()));
nodes.add("server", VPackValue(_server));
nodes.add("queryId", VPackValue(_queryId));

// And close it:
nodes.close();
Copy link
Member

Choose a reason for hiding this comment

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

Finally!
This was such a headache to remember every time...

}

/// @brief estimateCost
Expand Down Expand Up @@ -202,16 +198,10 @@ std::unique_ptr<ExecutionBlock> ScatterNode::createBlock(
std::move(executorInfos));
}

/// @brief toVelocyPack, for ScatterNode
void ScatterNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const {
// call base class method
ExecutionNode::toVelocyPackHelperGeneric(nodes, flags, seen);

/// @brief doToVelocyPack, for ScatterNode
void ScatterNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
// serialize clients
writeClientsToVelocyPack(nodes);
// And close it:
nodes.close();
}

bool ScatterNode::readClientsFromVelocyPack(VPackSlice base) {
Expand Down Expand Up @@ -312,23 +302,15 @@ std::unique_ptr<ExecutionBlock> DistributeNode::createBlock(
std::move(infos));
}

/// @brief toVelocyPack, for DistributedNode
void DistributeNode::toVelocyPackHelper(VPackBuilder& builder, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const {
/// @brief doToVelocyPack, for DistributeNode
void DistributeNode::doToVelocyPack(VPackBuilder& builder, unsigned flags) const {
// call base class method
ExecutionNode::toVelocyPackHelperGeneric(builder, flags, seen);

ScatterNode::doToVelocyPack(builder, flags);
// add collection information
CollectionAccessingNode::toVelocyPack(builder, flags);

// serialize clients
writeClientsToVelocyPack(builder);

builder.add(VPackValue("variable"));
_variable->toVelocyPack(builder);;

// And close it:
builder.close();
_variable->toVelocyPack(builder);
}

void DistributeNode::replaceVariables(std::unordered_map<VariableId, Variable const*> const& replacements) {
Expand Down Expand Up @@ -435,12 +417,8 @@ GatherNode::GatherNode(ExecutionPlan* plan, ExecutionNodeId id,
_parallelism(parallelism),
_limit(0) {}

/// @brief toVelocyPack, for GatherNode
void GatherNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const {
// call base class method
ExecutionNode::toVelocyPackHelperGeneric(nodes, flags, seen);

/// @brief doToVelocyPack, for GatherNode
void GatherNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
nodes.add("parallelism", VPackValue(toString(_parallelism)));

if (_elements.empty()) {
Expand All @@ -467,9 +445,6 @@ void GatherNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
}
}
}

// And close it:
nodes.close();
}

/// @brief creates corresponding ExecutionBlock
Expand Down Expand Up @@ -656,11 +631,8 @@ std::unique_ptr<ExecutionBlock> SingleRemoteOperationNode::createBlock(
}
}

/// @brief toVelocyPack, for SingleRemoteOperationNode
void SingleRemoteOperationNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const {
// call base class method
ExecutionNode::toVelocyPackHelperGeneric(nodes, flags, seen);
/// @brief doToVelocyPack, for SingleRemoteOperationNode
void SingleRemoteOperationNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
CollectionAccessingNode::toVelocyPackHelperPrimaryIndex(nodes);

// add collection information
Expand Down Expand Up @@ -703,9 +675,6 @@ void SingleRemoteOperationNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned
nodes.add("projections", VPackValue(VPackValueType::Array));
// TODO: support projections?
nodes.close();

// And close it:
nodes.close();
}

/// @brief estimateCost
Expand Down
39 changes: 19 additions & 20 deletions arangod/Aql/ClusterNodes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,6 @@ class RemoteNode final : public DistributeConsumerNode {
/// @brief return the type of the node
NodeType getType() const override final { return REMOTE; }

/// @brief export to VelocyPack
void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const override final;

/// @brief creates corresponding ExecutionBlock
std::unique_ptr<ExecutionBlock> createBlock(
ExecutionEngine& engine,
Expand Down Expand Up @@ -112,6 +108,10 @@ class RemoteNode final : public DistributeConsumerNode {
_queryId = arangodb::basics::StringUtils::itoa(queryId);
}

protected:
/// @brief export to VelocyPack
void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override final;

private:
/// @brief the underlying database
TRI_vocbase_t* _vocbase;
Expand All @@ -137,10 +137,6 @@ class ScatterNode : public ExecutionNode {
/// @brief return the type of the node
NodeType getType() const override { return SCATTER; }

/// @brief export to VelocyPack
void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const override;

/// @brief creates corresponding ExecutionBlock
std::unique_ptr<ExecutionBlock> createBlock(
ExecutionEngine& engine,
Expand Down Expand Up @@ -176,6 +172,9 @@ class ScatterNode : public ExecutionNode {
void setScatterType(ScatterType targetType) { _type = targetType; }

protected:
/// @brief export to VelocyPack
void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override;

void writeClientsToVelocyPack(velocypack::Builder& builder) const;
bool readClientsFromVelocyPack(velocypack::Slice base);

Expand Down Expand Up @@ -205,10 +204,6 @@ class DistributeNode final : public ScatterNode, public CollectionAccessingNode
/// @brief return the type of the node
NodeType getType() const override final { return DISTRIBUTE; }

/// @brief export to VelocyPack
void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const override final;

/// @brief creates corresponding ExecutionBlock
std::unique_ptr<ExecutionBlock> createBlock(
ExecutionEngine& engine,
Expand All @@ -232,6 +227,10 @@ class DistributeNode final : public ScatterNode, public CollectionAccessingNode

ExecutionNodeId getTargetNodeId() const noexcept { return _targetNodeId; }

protected:
/// @brief export to VelocyPack
void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override final;

private:
/// @brief the variable we must inspect to know where to distribute
Variable const* _variable;
Expand Down Expand Up @@ -274,10 +273,6 @@ class GatherNode final : public ExecutionNode {
/// @brief return the type of the node
NodeType getType() const override final { return GATHER; }

/// @brief export to VelocyPack
void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const override final;

/// @brief clone ExecutionNode recursively
ExecutionNode* clone(ExecutionPlan* plan, bool withDependencies,
bool withProperties) const override final {
Expand Down Expand Up @@ -320,6 +315,10 @@ class GatherNode final : public ExecutionNode {
return _parallelism;
}

protected:
/// @brief export to VelocyPack
void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override final;

private:
/// @brief the underlying database
TRI_vocbase_t* _vocbase;
Expand Down Expand Up @@ -364,10 +363,6 @@ class SingleRemoteOperationNode final : public ExecutionNode, public CollectionA
/// @brief return the type of the node
NodeType getType() const override final { return REMOTESINGLE; }

/// @brief export to VelocyPack
void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const override final;

/// @brief creates corresponding ExecutionBlock
std::unique_ptr<ExecutionBlock> createBlock(
ExecutionEngine& engine,
Expand Down Expand Up @@ -396,6 +391,10 @@ class SingleRemoteOperationNode final : public ExecutionNode, public CollectionA

std::string const& key() const { return _key; }

protected:
/// @brief export to VelocyPack
void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override final;

private:
// whether we replaced an index node
bool _replaceIndexNode;
Expand Down
11 changes: 2 additions & 9 deletions arangod/Aql/CollectNode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,8 @@ CollectNode::CollectNode(

CollectNode::~CollectNode() = default;

/// @brief toVelocyPack, for CollectNode
void CollectNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const {
// call base class method
ExecutionNode::toVelocyPackHelperGeneric(nodes, flags, seen);

/// @brief doToVelocyPack, for CollectNode
void CollectNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
// group variables
nodes.add(VPackValue("groups"));
{
Expand Down Expand Up @@ -148,9 +144,6 @@ void CollectNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
nodes.add("specialized", VPackValue(_specialized));
nodes.add(VPackValue("collectOptions"));
_options.toVelocyPack(nodes);

// And close it:
nodes.close();
}

void CollectNode::calcExpressionRegister(arangodb::aql::RegisterId& expressionRegister,
Expand Down
8 changes: 4 additions & 4 deletions arangod/Aql/CollectNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,6 @@ class CollectNode : public ExecutionNode {
/// @brief getOptions
CollectOptions& getOptions();

/// @brief export to VelocyPack
void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const override final;

/// @brief calculate the expression register
void calcExpressionRegister(RegisterId& expressionRegister,
RegIdSet& writeableOutputRegisters) const;
Expand Down Expand Up @@ -187,6 +183,10 @@ class CollectNode : public ExecutionNode {
static void calculateAccessibleUserVariables(ExecutionNode const& node,
std::vector<Variable const*>& userVariables);

protected:
/// @brief export to VelocyPack
void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override final;

private:
/// @brief options for the aggregation
CollectOptions _options;
Expand Down
13 changes: 1 addition & 12 deletions arangod/Aql/DistributeConsumerNode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,7 @@ DistributeConsumerNode::DistributeConsumerNode(ExecutionPlan* plan,
_isResponsibleForInitializeCursor(
base.get("isResponsibleForInitializeCursor").getBoolean()) {}

void DistributeConsumerNode::toVelocyPackHelper(VPackBuilder& nodes, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const {
// Local variant
toVelocyPackHelperInternal(nodes, flags, seen);
// And close it:
nodes.close();
}

void DistributeConsumerNode::toVelocyPackHelperInternal(
VPackBuilder& nodes, unsigned flags, std::unordered_set<ExecutionNode const*>& seen) const {
// call base class method
ExecutionNode::toVelocyPackHelperGeneric(nodes, flags, seen);
void DistributeConsumerNode::doToVelocyPack(VPackBuilder& nodes, unsigned flags) const {
nodes.add("distributeId", VPackValue(_distributeId));
nodes.add("isResponsibleForInitializeCursor", VPackValue(_isResponsibleForInitializeCursor));
}
Expand Down
8 changes: 2 additions & 6 deletions arangod/Aql/DistributeConsumerNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@ class DistributeConsumerNode : public ExecutionNode {
/// @brief return the type of the node
NodeType getType() const override { return DISTRIBUTE_CONSUMER; }

/// @brief export to VelocyPack
void toVelocyPackHelper(arangodb::velocypack::Builder&, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const override;

/// @brief creates corresponding ExecutionBlock
std::unique_ptr<ExecutionBlock> createBlock(
ExecutionEngine& engine,
Expand Down Expand Up @@ -92,8 +88,8 @@ class DistributeConsumerNode : public ExecutionNode {
}

protected:
void toVelocyPackHelperInternal(arangodb::velocypack::Builder& nodes, unsigned flags,
std::unordered_set<ExecutionNode const*>& seen) const;
/// @brief export to VelocyPack
void doToVelocyPack(arangodb::velocypack::Builder&, unsigned flags) const override;

private:
/// @brief our own distributeId. If it is set, we will fetch only ata for
Expand Down
Loading