Summary:
Refactor in two ways. First, expose members without getters as we will
need most of them in distributed. And this was always the sensible thing
to do. Second, add storage type values to deltas. This is also a
sensible thing to do, and it will be very beneficial in distributed. We
didn't do it before because name<->value type mappings aren't guaranteed
to be the same after recovery. A task has been added to address this
(preserve mappings in durability).
Reviewers: dgleich, buda
Reviewed By: dgleich
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1167
Summary:
Remote pulls can now be async. Note that
`RemotePullRpcClients::RemotePull` takes references to data structures
which should not be temporary in the caller. Still, maybe safer to make
copies?
Changed `RpcWorkerClients` API to make that possible.
Reviewers: dgleich, msantl, teon.banek
Reviewed By: msantl
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1150
Summary:
It is possible that we have a global address to resolve, for a graph
element that's local. Consider W1 expanding, getting data from W2,
expanding from there and getting data that is on W1. We then don't want
to do RPC from W1 to W1, but do a lookup directly.
Reviewers: dgleich
Reviewed By: dgleich
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1145
Summary:
NOTE: This diff is still in progress. Many TODOs, lacking documentation
etc. But the main logic is there (some could be different), and it tests
out OK.
Reviewers: teon.banek, msantl, buda
Reviewed By: teon.banek
Subscribers: mferencevic, pullbot
Differential Revision: https://phabricator.memgraph.io/D1138
Summary:
With this patch the number of packets for a simple RPC call is lowered
from 22 to 12 (45% reduction). The number of packets for the Bolt protocol
is lowered from 26 to 18 (30% reduction).
Impact on the Bolt protocol will be a constant of ~ 8 packets less per
connection, while the impact on the RPC protocol will be approximately
a 45% reduction overall.
Reviewers: buda, teon.banek
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1141
Summary: See above. The unit test creates two clients on demand so I guess it works.
Reviewers: mferencevic, florijan, teon.banek
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1136
Summary:
Start removal of old logic
Remove more obsolete classes
Move Message class to RPC
Remove client logic from system
Remove messaging namespace
Move protocol from messaging to rpc
Move System from messaging to rpc
Remove unnecessary namespace
Remove System from RPC Client
Split Client and Server into separate files
Start implementing new client logic
First semi-working state
Changed network protocol layout
Rewrite client
Fix client receive bug
Cleanup code of debug lines
Migrate to accessors
Migrate back to binary boost archives
Remove debug logging from server
Disable timeout test
Reduce message_id from uint64_t to uint32_t
Add multiple workers to server
Fix compiler warnings
Apply clang-format
Reviewers: teon.banek, florijan, dgleich, buda, mtomic
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1129
Summary:
Added test for `PlanDispatcher` and `PlanConsumer`.
This diff also contains a fix for the async rpc call on all clients.h
Reviewers: florijan, teon.banek
Reviewed By: florijan
Subscribers: pullbot, dgleich
Differential Revision: https://phabricator.memgraph.io/D1135
Summary:
- End to end distributed GraphDb testing
- Refactors as necessary
- Basic RemoteCache for storing remote data
- RemoteDataRpc
As we are on a tight schedule, please let's focus on the essentials:
functionality and proper testing.
Reviewers: dgleich, teon.banek, buda
Reviewed By: dgleich
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1121
Summary:
With the added support for serialization, we should be able to transfer
plans across distributed workers.
The planner tests has been extended to test serialization. Operators
should be mostly tested, but the expression they contain aren't
completely. The quick solution is to use typeid for rudimentary
expression equality testing. The more involved solution of comparing
the expression tree for equality is the correct choice. It should be
done in the near future.
Reviewers: florijan, msantl
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1122
Summary:
Other than the plan operators and the frame, we will need to pass the
generated symbol table to distributed workers.
Reviewers: florijan, msantl
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1123
Summary:
Adds worker id to snapshot and wal filename.
Adds a new worker_id flag to be used for recovering a worker with a distributed snapshot.
Adds worker_id field to snapshot to check for consistency.
Reviewers: florijan
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1096
Summary:
Previously, we would have a `DCHECK` which crashes the application. This
was evident when testing a queries, such as:
MATCH (n) DELETE n SET n.prop = 42
Since the argument to update clauses is evaluated during execution, it
makes it very difficult to prevent such errors during semantic analysis.
For example:
MATCH (n)--(m) WITH collect(n) as ns, m
DETACH DELETE ns[m.prop] SET head(ns).prop = 42
Test query updates on deleted graph elements
Reviewers: florijan, dgleich
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1114
Summary:
Close the file descriptor in File destructor. This will prevent
accidental crashes during unexpected destructor calls. For example, if
an exception is thrown before the file is closed. File now takes
ownership of the descriptor. These changes now honor RAII idiom, which
should handle most of the peculiarities of C++.
Use optional value for TryOpenFile function, instead of returning a File
without a descriptor. It makes the failure state more semantically clear
to the API user.
Merge utils/filesystem with utils/file
The files aren't that big, and the naming is a bit confusing because
functions aren't really grouped for file and filesystem distinction.
Reviewers: mferencevic, mtomic
Reviewed By: mferencevic
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1111
Summary:
Added wrappers for some Unix system calls in utils/filesystem.hpp and implemented
a simple log storage interface for Raft. It is not very efficient, we will need
something more sophisticated later, but this is good enough for testing.
Reviewers: mferencevic, mislav.bradac, buda, mculinovic
Reviewed By: mferencevic
Subscribers: teon.banek, dgleich, pullbot
Differential Revision: https://phabricator.memgraph.io/D1091
Summary:
The directory was never actually copied on apollo, so tests weren't even
doing anything...
Also remove fswatcher unit test, it should be rewritten correctly.
Reviewers: mislav.bradac, mferencevic, buda
Reviewed By: mferencevic
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1108
Summary:
GraphDb is refactored to become an API exposing different parts
necessary for the database to function. These different parts can have
different implementations in SingleNode or distributed Master/Server
GraphDb implementations.
Interally GraphDb is implemented using two class heirarchies. One
contains all the members and correct wiring for each situation. The
other takes care of initialization and shutdown. This architecture is
practical because it can guarantee that the initialization of the
object structure is complete, before initializing state.
Reviewers: buda, mislav.bradac, dgleich, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1093
Summary:
No logic changes, just split `tx::MasterEngine` into
`tx::SingleNodeEngine` and `tx::MasterEngine`. This gives better
responsibility separation and is more appropriate now there is no
Start/Shutdown.
Reviewers: dgleich, teon.banek, buda
Reviewed By: dgleich, teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1099
Summary:
Serialization of vertices and edges for distributed. Based on Boost
serialization. Threrefore moved TypedValue serialization from AST to
utils.
Reviewers: buda, dgleich, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1088
Summary:
A PropertyValueStore is not a generic data structure, but only ever used
to store properties in a Vertex/Edge. It has behaviours specific to it.
So, the templatization was not necessary.
Reviewers: buda
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1089
Summary:
Although the first solution used cereal, the final implementation uses
boost. Since the cereal is still used in the codebase, compilation has
been modified to support multithreaded cereal.
In addition to serializing Ast classes, the following also needed to be
serialized:
* GraphDbTypes
* Symbol
* TypedValue
TypedValue is treated specially, by inlining the serialization code in
the Ast class, concretely PrimitiveLiteral.
Another special case was the Function Ast class, which now stores a
function name which is resolved to a concrete std::function on
construction.
Tests have been added for serialized Ast in
tests/unit/cypher_main_visitor
Reviewers: mferencevic, mislav.bradac, florijan
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1067
Summary:
The distributed ID mapper is not yet utilised in GraphDb as those
changes are in D1060. Depending on landing order it will be added.
Reviewers: dgleich, mislav.bradac
Reviewed By: dgleich
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1064
Summary:
Implement AddComand method on RaftMember
Move RPCType out of rpc request and reply
Add unit test for AddCommand
Reviewers: mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot, mculinovic
Differential Revision: https://phabricator.memgraph.io/D1042
Summary:
Special casing the last token when adding it to the named expression list.
Added a test to check this case.
Reviewers: mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1052
Summary:
Rpc client wasn't thread safe and required a lock before each rpc call.
The locking functionality is now incorporated in Rpc client.
Reviewers: mislav.bradac
Reviewed By: mislav.bradac
Differential Revision: https://phabricator.memgraph.io/D1056
Summary: Add curvy braces handling in `QueryStripper` and an accompanying test for it.
Reviewers: mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1051
Summary: In this diff I just wanted to fix tests' flakyness. We can discuss if we want to always pass endpoint as an argument and never pass address:port pair explicitly. However if we decide that, I will do that change in another diff.
Reviewers: dgleich, florijan
Reviewed By: dgleich, florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1049
Summary:
Union query combinator implementation consists of:
* adjustments to the AST and `cypher_main_visitor`
* enabling `QueryStripper` to parse multiple `return` statements (not stopping after first)
* symbol generation for union results
* union logical operator
* query plan generator adjustments
Reviewers: teon.banek, mislav.bradac
Reviewed By: teon.banek
Subscribers: pullbot, buda
Differential Revision: https://phabricator.memgraph.io/D1038
Summary: RPC recipient should update its term even if it is rejecting the request.
Reviewers: mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1039
Summary:
Implement log replication
Rebase and fix src/CMakeLists.txt
Some style fixes
Changed shared_ptr to unique_ptr for RaftPeerState
Change Id and Leader to const
Move implementation to separate class
Fix raft_experiments.cpp
Reviewers: mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1033
Summary:
What's done:
- `RecordAccessor` can represent remote data
- `GraphDbAccessor` manages remote data
- Cleanup: different `EdgeAccessor lazyness (@dgleich: take a look), unused methods, documentation...
- `TODO` placeholders for remote implementation
What's not done:
- RPC and data transfer
- how exactly remote errors are handled
- not sure if any MVCC Record info for remote data should be tracked
- WAL and RPC Deltas properly handled (Gleich working on extracting `Wal::Op`)
This implementation should not break single-node execution, and should provide good abstractions and placeholders for distributed. Once that's satisfied, it should land.
Reviewers: dgleich, buda, mislav.bradac
Reviewed By: dgleich
Subscribers: dgleich, pullbot
Differential Revision: https://phabricator.memgraph.io/D1030
Summary: Operations are moved and renamed from WAL to a separate file in preparation for HA and distributed storage.
Reviewers: florijan, mtomic, mislav.bradac
Reviewed By: florijan
Subscribers: mislav.bradac, pullbot
Differential Revision: https://phabricator.memgraph.io/D1034
Summary:
It occurred that part of the durability flakyness test might be that the
same durability directory is used always. If the test is run
simultaneously on a single system, there will be interference.
This might not actually fix all the flakyness :(
I also made the `utils::RandomString` function since that's now used in
multiple places, tested it etc.
Reviewers: buda, dgleich
Reviewed By: dgleich
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1020
Summary:
This change generates multiple PropertyFilters for expressions such as
`n.prop1 = m.prop2`. When choosing one PropertyFilter, we want to also
remove the other one, because they represent the same original
expression. Therefore, the removal is no longer based on FilterInfo
equality, but on the original expression equality. Additionally,
FilterInfo and PropertyFilter equality operators have been removed to
avoid any pretense they do what you expect or want.
Reviewers: florijan, msantl
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1021
Summary:
The current idea is that the same MG binary can be used for single-node,
distributed master and distributed worker. The transactional engine in
the single-node and distributed master is the same: it determines the
transactional time and exposes all the "global" functionalities. In the
distributed worker the "global" functions must contact the master.
Reviewers: dgleich, mislav.bradac, buda
Reviewed By: dgleich
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1013
Summary: Because it will never be used, we already have replacements for it.
Reviewers: buda
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1016
Summary: Once a snapshot is successfully written, delete WAL files which are no longer necessary for recovery. Note that this prohibits recovering the WAL from any except the last snapshot.
Reviewers: buda, mislav.bradac, dgleich
Reviewed By: dgleich
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1000
Summary:
This diff contains step 1:
- Remove clog exposure from tx::engine
- Reduce and cleanup tx::Engine API
All current functionality is kept, but the API is reduced. This is very
desirable because every function in tx::Engine will need to be
considered and implemented in both Master and Worker situations. The
less we have, the better.
Next step is exactly that: seeing how each of these functions behaves in
a distributed system and implementing accordingly.
Reviewers: dgleich, mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1008
Summary:
Referring to the TCK failure on:
```
MATCH (a {name: 'Andres'})<-[:FATHER]-(child)
RETURN {foo: a.name='Andres', kids: collect(child.name)}
```
In the planner we'd only treat a list|map as a group_by if it contained
no aggregations. That's changed so that if a map contains both aggregations
and non-aggregations, then non-aggregations are treated as individual
group_by expressions.
Reviewers: teon.banek
Reviewed By: teon.banek
Subscribers: buda, pullbot, teon.banek
Differential Revision: https://phabricator.memgraph.io/D1004
Summary:
In preparation for distributed storage we need to have labels/properties/edgetypes uniquely identifiable by their ids, which will be global in near future.
The old design has to be abandoned because it's not possible to keep track of global labels/properties/edgetypes while they are local pointers.
Reviewers: mislav.bradac, florijan
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D993
Summary:
Looking for connected components in a random graph. This test performs the following:
- Generates a random graph that is NOT sequential in memory (otherwise itertion over edges is 2 or more times faster).
- Connectivity by iterating over all the edges.
- Ditto over vertices.
- Ditto over vertices in parallel.
Not done:
- Edge filtering based on XY. I could/should add that to see how it affects perf.
- Getting component info out from union-find.
Local results are encouraging. Iterating over the graph is the bottleneck. Still, I get connectivity of 10M vertices/edges in <7sec (parallel over vertices). Will test on 250M remote now.
Locally obtained results (20M/20M, 2 threads)
```
I1115 14:57:55.136875 357 otto_parallel.cpp:50] Generating 2000000 vertices...
I1115 14:58:19.057734 357 otto_parallel.cpp:74] Generated 2000000 vertices in 23.9208 seconds.
I1115 14:58:19.919221 357 otto_parallel.cpp:82] Generating 2000000 edges...
I1115 14:58:39.519951 357 otto_parallel.cpp:93] Generated 2000000 edges in 19.3398 seconds.
I1115 14:58:39.520349 357 otto_parallel.cpp:196] Running Edge iteration...
I1115 14:58:43.857264 357 otto_parallel.cpp:199] Done in 4.33691 seconds, result: 3999860270398
I1115 14:58:43.857316 357 otto_parallel.cpp:196] Running Vertex iteration...
I1115 14:58:49.498181 357 otto_parallel.cpp:199] Done in 5.64087 seconds, result: 4000090070787
I1115 14:58:49.498208 357 otto_parallel.cpp:196] Running Connected components - Edges...
I1115 14:58:54.232530 357 otto_parallel.cpp:199] Done in 4.73433 seconds, result: 323935
I1115 14:58:54.232570 357 otto_parallel.cpp:196] Running Connected components - Vertices...
I1115 14:59:00.412395 357 otto_parallel.cpp:199] Done in 6.17983 seconds, result: 323935
I1115 14:59:00.412422 357 otto_parallel.cpp:196] Running Parallel connected components - Vertices...
I1115 14:59:04.662087 357 otto_parallel.cpp:199] Done in 4.24967 seconds, result: 323935
I1115 14:59:04.662116 357 otto_parallel.cpp:196] Running Expansion...
I1115 14:59:13.913015 357 otto_parallel.cpp:199] Done in 9.25091 seconds, result: 323935
```
Reviewers: buda, mislav.bradac, dgleich, teon.banek
Reviewed By: buda, teon.banek
Subscribers: teon.banek, pullbot
Differential Revision: https://phabricator.memgraph.io/D982
Summary: Vertex and Edge now use Address for storing connections to other Edges and Vertices, to support distributed storage.
Reviewers: mislav.bradac, dgleich, buda
Reviewed By: mislav.bradac, dgleich
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D977
Summary:
My dear fellow Memgraphians. It's friday afternoon, and I am as ready to pop as WAL is to get reviewed...
What's done:
- Vertices and Edges have global IDs, stored in `VersionList`. Main storage is now a concurrent map ID->vlist_ptr.
- WriteAheadLog class added. It's based around buffering WAL::Op objects (elementraly DB changes) and periodically serializing and flusing them to disk.
- Snapshot recovery refactored, WAL recovery added. Snapshot format changed again to include necessary info.
- Durability testing completely reworked.
What's not done (and should be when we decide how):
- Old WAL file purging.
- Config refactor (naming and organization). Will do when we discuss what we want.
- Changelog and new feature documentation (both depending on the point above).
- Better error handling and recovery feedback. Currently it's all returning bools, which is not fine-grained enough (neither for errors nor partial successes, also EOF is reported as a failure at the moment).
- Moving the implementation of WAL stuff to .cpp where possible.
- Not sure if there are transactions being created outside of `GraphDbAccessor` and it's `BuildIndex`. Need to look into.
- True write-ahead logic (flag controlled): not committing a DB transaction if the WAL has not flushed it's data. We can discuss the gain/effort ratio for this feature.
Reviewers: buda, mislav.bradac, teon.banek, dgleich
Reviewed By: dgleich
Subscribers: mtomic, pullbot
Differential Revision: https://phabricator.memgraph.io/D958
Summary:
Previously, named path symbols remained untracked as `new_symbols` during planning. This meant that
operator `Optional` would be left unaware of those symbols, and therefore not reset them to `Null`
if optional matching failed.
Test Optional operator will be aware of path symbols
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D974
Summary:
Tests have been updated to catch this error and other behaviour. Other
than this change, `AND` should behave as before.
Reviewers: florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D970
Summary:
Use sigaction to register signal handlers.
This is preferred over `signal` function, according to `man 3p signal`.
Add global sig_atomic_t flag when shutting down.
Block other signal handlers when shutting down.
Reviewers: mislav.bradac, mferencevic
Reviewed By: mferencevic
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D943
Summary:
Remove name from GraphDb.
Take GraphDb in query test macros instead of accessor.
Add is_accepting_transactions flag to GraphDb.
Reviewers: mislav.bradac, florijan, mferencevic
Reviewed By: mislav.bradac
Subscribers: mferencevic, pullbot
Differential Revision: https://phabricator.memgraph.io/D940
Summary:
- Removed durability::Summary because it was wired into reader and stopped me from recovering WAL files.
- Refactored and renamed BufferedFile(Reader/Writer) to HashedFile(Reader/Writer).
- Vertex and edge counts in the snapshot are now hashed.
Breaking snapshot compatibility again (hashing), but since the previous version was not released, and we are not caching snapshots, the previous version does not need to be supported.
Reviewers: teon.banek, mislav.bradac, buda
Reviewed By: teon.banek, mislav.bradac
Subscribers: dgleich, pullbot
Differential Revision: https://phabricator.memgraph.io/D932
Summary:
Move QueryParts and Filters to a new file.
Reorganize FilterInfo struct.
Remove label filter if we do indexed scan by label.
Remove property filter used in indexed scan.
Reviewers: florijan
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D915
Summary: Locked version. There are some benchmarks, it seems the lock won't be the bottleneck in the WAL (DB ops causing WAL delta insertions into it will be slower, flushing the WAL be slower).
Reviewers: buda, mislav.bradac, dgleich
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D919
Summary:
New snapshot structure:
- magic number
- snapshot version (old-version recovery not yet implemented)
- transaction snapshot (will be used in the WAL)
- the rest is as before (indices, vertices, edges)
Not backward compatible with the old snapshotting.
Does not improve error handling (user feedback). A task for that has been added.
Reviewers: buda, mislav.bradac, mferencevic, teon.banek
Reviewed By: teon.banek
Subscribers: teon.banek, dgleich, pullbot
Differential Revision: https://phabricator.memgraph.io/D912
Summary: It wasn't used in MG, only in tests.
Reviewers: buda, dgleich, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D909
Summary: This change increases the planning time, but should reduce memory consumption.
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D901
Summary: In the current state, it was not possible to iterate, or even access a const map, or const set structure because of an incorrect implementation of "ConstAccessors".
Reviewers: mislav.bradac, teon.banek, buda
Reviewed By: teon.banek, buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D902
Summary:
This puts the whole installation and packaging under a single point of
entry. (Docker, DEB, RPM, etc.)
Rename alpha.dockerfile to beta.dockerfile
Use Debian Stretch for docker
Remove building old hardcoded compiler
Rename build_interpreter to build_memgraph
Remove unused config-file
Reviewers: mferencevic, buda
Reviewed By: mferencevic, buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D857
Summary:
- Removed BreadthFirstAtom, using EdgeAtom only with a Type enum.
- Both variable expansions (breadth and depth first) now have mandatory inner node and edge Identifiers.
- Both variable expansions use inline property filtering and support inline lambdas.
- BFS and variable expansion now have the same planning process.
- Planner modified in the following ways:
- Variable expansions support inline property filtering (two filters added to all_filters, one for inline, one for post-expand).
- Asserting against existing_edge since we don't support that anymore.
- Edge and node symbols bound after variable expansion to disallow post-expand filters to get inlined.
- Some things simplified due to different handling.
- BreadthFirstExpand logical operator merged into ExpandVariable. Two Cursor classes remain and are dynamically chosen from.
As part of planned planner refactor we should ensure that a filter is applied only once. The current implementation is very suboptimal for property filtering in variable expansions.
@buda: we will start refactoring this these days. This current planner logic is too dense and complex. It is becoming technical debt. Most of the time I spent working on this has been spent figuring the planning out, and I still needed Teon's help at times. Implementing the correct and optimal version of query execution (avoiding multiple potentially expensive filterings) was out of reach also due to tech debt.
Reviewers: buda, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot, buda
Differential Revision: https://phabricator.memgraph.io/D852
Summary:
Add json and cppitertools to libs/CMakeLists.txt.
Import external projects as libraries.
This removes the need to use `add_dependencies` in order to link with
external project.
Extract common ExternalProject_Add function.
Add macro for easier addition of external libraries.
Reviewers: mislav.bradac, mferencevic
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D845
Summary:
- The new BFS syntax implemented as proposed.
- AST BreadthFirstAtom now uses EdgeAtom members: has_range_{true}, upper_bound_, lower_bound_
- Edges data structure now handles all the edge filtering (single or multiple edges), to ease planning. Additional edge filtering (additional Filter op in the plan) is removed. AST EdgeTypeTest is no longer used and is removed.
Current state is stable but there are things left to do:
- BFS property filtering.
- BFS lower_bound_ support.
- Support for lambdas in variable length expansion. This includes obligatory (even if not user_defined) inner_node and inner_edge symbols for easier handling.
- Code-sharing between BFS and variable length expansions.
I'll add asana tasks (and probably start working on them immediately) when/if this lands.
Reviewers: buda, teon.banek, mislav.bradac
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D836
Summary: - modified all utils/algorithm functions to be inline and in the utils namespace
Reviewers: teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D830
Summary:
I started with cleaning flags up (removing unused ones, documenting undocumented ones). There were some flags to remove in `QueryEngine`. Seeing how we never use hardcoded queries (AFAIK last Mislav's testing also indicated they aren't faster then interpretation), when removing those unused flags the `QueryEngine` becomes obsolete. That means that a bunch of other stuff becomes obsolete, along with the hardcoded queries. So I removed it all (this has been discussed and approved on the daily).
Some flags that were previously undocumented in `docs/user_technical/installation` are now documented. The following flags are NOT documented and in my opinion should not be displayed when starting `./memgraph --help` (@mferencevic):
```
query_vertex_count_to_expand_existsing (from rule_based_planner.cpp)
query_max_plans (rule_based_planner.cpp)
```
If you think that another organization is needed w.r.t. flag visibility, comment.
@teon.banek: I had to remove some stuff from CMakeLists to make it buildable. Please review what I removed and clean up if necessary if/when this lands. If the needed changes are minor, you can also comment.
Reviewers: buda, mislav.bradac, teon.banek, mferencevic
Reviewed By: buda, mislav.bradac
Subscribers: pullbot, mferencevic, teon.banek
Differential Revision: https://phabricator.memgraph.io/D825
Summary:
Test planner splits MATCH ... WHERE
Remove distinction between FilterAnd and AndOperator
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D814
Summary: Removed the traversal API, been waiting for named path to land because that data structure has replaced traversal's Path. I left the wiki page of the API, just put a warning that it's not used.
Reviewers: buda, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D809
Summary:
Three TODOs resolved.
1. around line 897 - we currently don't support expansion into existing variable length edges (there is a TODO in symbol_generator.cpp:213), so this should not be done at the moment.
2. around line 1025 - This TODO was on review and nobody commented, so I'm removing it. Should have done that when the diff landed.
3. around line 1560 - This does not seem possible. Edge-uniqueness checks happen within a single `[OPTIONAL ] MATCH`. If it is OPTIONAL (the case interesting here), then the uniqueness check also gets planned under the optional branch. So, if an optional fails, the uniqueness check will get skipped, as opposed to getting executed over a Null. I added an edge-case test to verify this (and checked with the planner test).
Reviewers: buda, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D807
Summary:
- Keys() functions in the indices can't be const because ConcurrentMap doesn't provide const accessors (and they are broken in skiplist) :D
- no cucumber tests because many tests create indices so it's hard to say what's inside and what not
Reviewers: buda, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D797
Summary:
This is done when the generated AST will be cached.
Remove LiteralsPlugger.
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D788
Summary:
Antlr grammar has been updated to support putting edge types after the BFS
symbol. Planner collects edge type filters for BFS and inlines them in the
operator by joining the filter with the user input BFS filter itself. This
requires no change from the standpoint of the operator. On the other hand, in
order to use the faster lookup by a single edge type, `ExpandBreadthFirst`
operator now accept an optional edge type. The edge type is passed from the
planner only if the user is filtering by a single type.
Unit tests as well as tck have been updated.
Reviewers: florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D777
Summary:
Add function First to utils.
Insert EdgeType into Expand during planning.
Reviewers: florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D769
Summary:
This fixes the problem when the test would fail on some PCs...
Notably, mine...
Reviewers: florijan, mferencevic, mislav.bradac, buda
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D770
Summary:
- The `Edges` data structure now handles common ops, including providing an iterator over edges whose "other" vertex is know.
- This should improve performance on dense_expand tests in the harness without other side-effects.
- query::plan::Expand operator modified not to check for existing-node stuff since that now gets handled by the `Edges` data structure.
- `Edges::Iterator` implemented only for const iterators since that suffices for now. Can implement non-const if the need arrises.
Reviewers: buda, mislav.bradac, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D763
Summary: `assert` function added. Useful to us for asserting DB state in harness tests. Potentially useful to the client for breaking out of a query as soon as a predicate fails, as opposed to collecting result and checking them client-side.
Reviewers: buda, mislav.bradac, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D750
Summary: Some TypedValue arithmetic ops threw exceptions for the wrong reasons, the op applicability type checks were wrong. Also the tests for that behavior were wrong. These are the fixes.
Reviewers: mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D724
Summary:
This allows for inserting dummy DbAccessor in tests. Unfortunate side
effect of this change is that the whole implementation had to be moved
from cpp to hpp.
Also templatize remaining RuleBasedPlanner implementation
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D704
Summary:
Added:
- map support in PropertyValue
- conversion of map TypedValue to PropertyValue if appropriate flag is set (undocumented because it's private)
- ordering of map PropertyValue in LabelPropertyIndex
- issue raised regarding list and value property modifications in storage (currently unsupported)
Maybe I missed some feature or whatever?
Reviewers: mislav.bradac, buda, teon.banek
Reviewed By: mislav.bradac, buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D692
Summary:
This is needed in cases when the planner decides to start expanding from
the other end.
Reviewers: mislav.bradac, florijan
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D681
Summary: Because it was unnecessary and also implemented wrong (if someone tried using a Schduler with something other then std::mutex, it would not compile). We can trivially add this if it ever becomes necessary.
Reviewers: buda, mislav.bradac
Reviewed By: buda, mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D666
Summary:
A single line (graph_db.cpp:109 in the new code) was missing. This should have been done in D355 (made by DGleich, approved by Flor AND Buda AND Mislav :D).
Converted a lambda to a method for convenience.
Reviewers: buda, dgleich, mislav.bradac
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D665
Summary: Changed on-scope-exit-mechanism from macro (with two auto-generated variables and an all-capturing lambda) to an explicitly created variable that takes an std::function argument.
Reviewers: buda, mislav.bradac, teon.banek
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D659
Summary: Not strictly neccessary, but it's been itching me. It took an hour.
Reviewers: buda, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D648
Summary:
- added only one function for getting the total (in + out) vertex degree, it's required for the Ravelin use-case
- specific `degree_in` and `degree_out` functions can be added as necessary
- also fixed random_graph_generator bug (needed it for testing)
Reviewers: buda, mislav.bradac
Reviewed By: buda, mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D652
Summary:
Now all ScanAll and Expand ops are covered by the cost estimator. For ScanAll with indices cost estimation is pretty good, for new Expand ops it is tragically bad (Expand to the power of expansion depth, plus arbitrary filtering). Static cost estimation is wrong wrong wrong.
Currently cost estimation of even trivial plans that use indices is wrong because the planner leaves filtering expressions that are implicitly handled by the index in the operator tree, IIRC. Tasking Teon to revise this, even though I'm not sure how bad an influence this has on cost estimation and it's use in plan choosing.
Reviewers: mislav.bradac, teon.banek, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D633
Summary:
This diff contains a bug fix for the expansion operators that are currently on dev.
More importantly, it proposes end-to-end testing for edge-cases for which it's a
pain to write single-phase tests. In my opinion this is OK, you're all reviewers so
you can comment.
The test relies on left-to-right query execution. We need this guarantee in tests
like this. I propose renaming "RuleBasedPlanner" to "LeftToRightPlanner" to make
this explicit. As Teon is not here at the moment, will make this a task/discussion.
Reviewers: buda, mislav.bradac, teon.banek, lion
Reviewed By: mislav.bradac
Subscribers: mferencevic, pullbot
Differential Revision: https://phabricator.memgraph.io/D626
Summary:
Not complete (but review can start):
- implementation should be done
- still need to finish tests
- documentation missing
Reviewers: mislav.bradac, teon.banek, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D616
Summary: Fixed bug for SkipList::position_and_count for an item lesser then all skiplist elements.
Reviewers: mislav.bradac
Reviewed By: mislav.bradac
Differential Revision: https://phabricator.memgraph.io/D629
Summary:
`UNWIND` can come before `MATCH`, so it needs to break query parts. If
it didn't, a query part would incorrectly grab all the matches and plan
them incorrectly. A test for such a case has been added.
Reviewers: florijan
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D598
Summary:
Add All expression to Ast
Evaluate All expression
Visit All and generate symbols
Handle All when collecting context during planning
Reviewers: florijan, mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D587
Summary:
Add EdgeList symbol type and check for redeclaration
The result of variable path is a list of edges, so the symbol type has
been added. In the future, we need to extend the type checker and the
type structure to have a generic list type.
We also currently do not support reusing an already bound symbol for a
variable path, so the SymbolGenerator will raise a redeclaration error.
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot, lion
Differential Revision: https://phabricator.memgraph.io/D574
Summary: A helper function added for transferring graph elements into some GraphDbAccessor.
Reviewers: mislav.bradac, buda, teon.banek
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D581
Summary:
Variable expansion logical operator added. Some functionalities are missing:
- taking into account optional matching when expanding into existing symbol
- accepting Expression bounds (current implementation takes size_t)
Also, a TODO is added for handling optional matching in the uniqueness operator (with an Asana task)
All this will be done in the following diff, this is already substantial.
Also, please consider if we want to have all those `VLOG`s in the code. Not very pretty. And I think that `VLOG` is not compiled-away in release build, will put an asana task.
Reviewers: teon.banek, mislav.bradac, buda
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D569
Summary:
Allow expressions for variable length path bounds
Replace test which expected a syntax exception
Since we now allow variable length to have an arbitrary expression, the
test case is obsolete. It was replaced with something that excepts an
expression which wasn't allowed before.
Reviewers: florijan, mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: lion, pullbot
Differential Revision: https://phabricator.memgraph.io/D568
Summary:
- GraphDbAccessor - index range API added
- index api tests refactored
- skiplist minor cleanup.
Reviewers: teon.banek, buda, mislav.bradac
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D533
Summary:
Add optional bounds to PropertyFilter and collect them
Relation operators (e.g. `<`, `>` ...) should be used to produce
scanning the index by a range of values. For that reason, PropertyFilter
is extended to store either the equality expression or range bounds.
The `AnalyzeFilter` function is extended to look for those operators and
see if their top level expression contains a property lookup. If it
does, a filter with a bound is generated.
Test for property comparison preventing index use
Reviewers: florijan, mislav.bradac, buda
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D529
Summary:
There was only two files in dbms directory so I moved them to database
directory.
Reviewers: buda
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D540
Summary:
Add Filters class for storing additional info
Add FindOr to utils/algorithm.hpp
Use all collected labels when scanning by them
Collect label filters inside WHERE
Document the Filters class
Reviewers: florijan, mislav.bradac, buda
Reviewed By: florijan
Subscribers: pullbot, lion
Differential Revision: https://phabricator.memgraph.io/D515
Summary:
- added functionality to `GraphDbAccessor` for cardinality estimates
- changed all `GraphDbAccessor::Count...` functions to return `int64_t`
- added the need functionality into `LabelPropertyIndex`
- modified `SkipList::position_and_count` to accept a custom `equals` function. Equality could not be implemented using only the custom `less` because it compares a templated `TItem` with skiplist element type `T`, and is therefore not symetrical.
Reviewers: teon.banek, buda, mislav.bradac
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D521
Summary:
- refactored so `less` is used instead of `greater`
- added a fuzzy unit test
Reviewers: mislav.bradac, buda, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D519
Summary:
Add ScanAllByLabelPropertyRange operator
This operator uses the label + property indexing feature to iterate over
the vertices. The property value of each vertex is checked whether it is
inside the given range of values. The range is inclusive from both
sides. If the value isn't in range, the vertex is filtered out.
This manual filtering should be replaced by a database API when it
becomes available.
Add ScanAllByLabelPropertyValue operator
Reviewers: florijan, mislav.bradac, buda
Reviewed By: florijan, mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D503
Summary:
Remove utils/stacktrace/log.hpp
The single function defined in log.hpp is used only in
memgraph_bolt.cpp. Therefore, the function has been moved and the file
removed.
Move utils/stacktrace/stacktrace.hpp one level up
Move some logging from memgraph_bolt to Server
Reviewers: buda, dtomicevic
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D465
Summary:
This is an obvious bug, caused by an oversight. A test has been added
for this case.
Reviewers: florijan, mislav.bradac, buda
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D455
Summary:
- GC changed to evaluate old records w.r.t. the oldest transaction's id AND snapshot, as opposed to only id
- MVCC hints exp+aborted race condition prevented
- minor MVCC refactors and cleanups
- minor Transaction refactors and cleanups
Reviewers: buda, dgleich
Reviewed By: buda, dgleich
Subscribers: dtomicevic, pullbot
Differential Revision: https://phabricator.memgraph.io/D434
Summary:
CMake is smart enough to transitively detect dependencies and link them
appropriately. Therefore, it is enough that we put all libraries that
memgraph uses to the dependency list of memgraph_lib and memgraph_pic
targets.
Patch the fmt library for C++14 and higher
fmt library would detect that C++11 is supported and then put the
compiler flag. This flag was set so it overrides parent project compiler
flags. This override from fmt would prevent us from using C++14
features. New version (3.1) of fmt resolves this issue, but it hasn't
been released yet. Therefore, this commit updates the script which
clones fmt to use the released 3.0.1 version and apply the fix on that.
Reviewers: dgleich, buda
Reviewed By: buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D441
Summary:
This fixes an issue when aggregations and/or group by expressions
weren't picked up from certain operators. In addition to that, we would
segfault in cases when the `has_aggregation_` is empty. For example,
function calls without arguments: `RETURN PI()`.
Test aggregations inside some operators
Reviewers: florijan, mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D425
Summary:
Permute query parts.
Permute matching only by selecting the starting node.
Flip the expansion when expanding from the other node.
Split planner into rule_based_planner and variable_start_planner
Use symbol hash when collecting expansion nodes
Multiple node atoms may point to the same symbol, and we could generate
multiple starting positions per atom which are the same. Using symbol
hash and equality prevents generating those redundant plans.
Correctly permute optional and merge matchings
Test VariableStartPlanner
Reviewers: florijan, mislav.bradac, buda, lion
Reviewed By: florijan, buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D417
Summary:
This change modifies the planning API to be more general, in order to support
picking different planning strategies. The current planning strategy has been
named RuleBasedPlanner.
Reviewers: florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D411
