Use `DeltaTriplesManager` to process `update` request #1608

Qup42 · 2024-11-07T13:12:51Z

Add the missing code in Server.cpp and Server.h to process a parsed update request using the DeltaTriplesManager from #1603 .
As of this commit, QLever has an initial beta support for a subset of SPARQL UPDATE with the following limitations (all of which will be added in the future, and the list is far from being complete):

Updates are not yet persistent, so a restart or crash of the QLever server will delete all updates.
Only a single update request per query is allowed, not the syntax that atomically chains an arbitrary sequence of updates.
Only INSERT and DELETE queries are supported, the support for queries that drop or add a complete graph will be added in the future.

joka921

Some minor requests and thoughts.
We should now have almost everything in place to try out merging all this together for experimenting.

src/engine/Server.cpp

joka921

Some small changes.

src/engine/Server.h

src/engine/Server.cpp

joka921 · 2024-11-08T10:17:42Z

src/engine/Server.cpp

+  auto [queryHub, messageSender] =
+      createMessageSender(queryHub_, request, update);
+
+  auto [cancellationHandle, cancelTimeoutOnDestruction] =
+      setupCancellationHandle(messageSender.getQueryId(), timeLimit);
+
+  auto [pinSubtrees, pinResult] = determineResultPinning(params);
+  LOG(INFO) << "Processing the following SPARQL update:"
+            << (pinResult ? " [pin result]" : "")
+            << (pinSubtrees ? " [pin subresults]" : "") << "\n"
+            << update << std::endl;
+  QueryExecutionContext qec(index_, &cache_, allocator_,
+                            sortPerformanceEstimator_, std::ref(messageSender),
+                            pinSubtrees, pinResult);
+  auto plannedQuery = co_await setupPlannedQuery(
+      params, update, qec, cancellationHandle, timeLimit, requestTimer);
+  auto qet = plannedQuery.queryExecutionTree_;


This code is identically duplicated between the query and the update case (the only differense is "UPDATE" vs "QUERY" in the Log message.
Just make it a function that
returns unique_ptr<SomeStructThatcontainsStuffThatWeNeedToAccessAndKeepAlive> and does all this common setup of message senders, cancellation handles, qets, and whatNotElse.

I am struggling with the type of cancelTimeoutOnDestruction lambda. Can you help with that @joka921? If skip the first two statements.

What I tried so far:

std::function - lambda is not copy constructible, because it captures

std::move_only_function - is c++23

function pointer - lambda captures

auto in the struct - lambda captures

obtaining the type through template magic was also difficult because the function is a private member function in the same class

TODO: This does not yet work for concurrent update requests (which need to be serialized via a lock). But good enough for playing around already.

hannahbast · 2024-11-09T12:17:50Z

@Qup42 I am already playing around with this (in conjunction with #1597) and it works like a charm. I have one request:

Currently, the API call simply returns a string "Update successful". It would be great if for Accept: application/qlever-results+json, it would return a JSON with (for now) the following fields:

A field time with the following subfields, all values in milliseconds: parse (the time for parsing the query), where (the time for evaluating the WHERE clause, or zero if there is none), update (the time for adding the triples to the delta triples and locating them in the blocks), and total (the total time for the update operation).
A field delta-triples with the following subfields: before (the number of delta triples before the update operation), after (the number of delta triples after the update operation), and difference (the value of after minus the value of before).
A field located-triples with the following subfields: pos with two subfields blocks-total (the total number of blocks in that permutation) and blocks-affected (the number of blocks with at least one located triple), and the same for every other permutation.

I am assuming that all this information is available relatively easily. If not, please let me know. Please also let me know if you think that this belongs in a separate PR.

And somewhat related: In my original (by now, ancient) draft of this, there was a command to clear all delta triples. For example, it could be activated via cmd=clear-delta-triples (it should require the access-token, of course). Would that be easy to add here or should that (also) be in a separate PR?

codecov · 2024-11-10T20:10:17Z

Codecov Report

Attention: Patch coverage is 7.69231% with 108 lines in your changes missing coverage. Please review.

Project coverage is 89.21%. Comparing base (1bcfeeb) to head (42f3846).
Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
src/engine/Server.cpp	6.95%	107 Missing ⚠️
src/index/DeltaTriples.cpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1608      +/-   ##
==========================================
- Coverage   89.21%   89.21%   -0.01%     
==========================================
  Files         372      374       +2     
  Lines       34723    35387     +664     
  Branches     3915     3992      +77     
==========================================
+ Hits        30979    31569     +590     
- Misses       2471     2529      +58     
- Partials     1273     1289      +16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Qup42 · 2024-11-11T12:51:16Z

@hannahbast adding some timing information and further metadata to the update response is already on my ToDo list. Points 1 (time) and 2 (delta-triples) should be no problem. I would extend the delta-triples field by the number of added/removed fields, as these are not knowable otherwise. For located-triples I can't estimate how hard this is.

My current plan was to do the additional info in a separate PR. The PR activating the updates in the server was supposed to be small, so that it can be merged quickly. Before adding this additional information I wanted to have a look at what other engines do here. To be able to run updates some changes are required in qlever-ui, so there was little use to every it earlier at that poing.
But this can of course change depending on e.g. how long it will take to merge #1597 and how much value the additional info would provide for you for while benchmarking.

Clearing the updates is just very little code, I'll add that to this PR.

…y updates

joka921

The general setup is fine, but there's some things to consider concerning threadsafety and
deadlock prevention.

src/index/DeltaTriples.cpp

src/engine/Server.cpp

joka921

Two very minor changes.

src/engine/Server.cpp

Signed-off-by: Johannes Kalmbach <[email protected]>

sparql-conformance · 2024-11-15T15:24:13Z

Conformance check passed ✅

No test result changes.

Details: https://qlever.cs.uni-freiburg.de/sparql-conformance-ui?cur=42f38469a38e057faa74e9071209e447b4759ccd&prev=77ac964e9de37c10142bc19735d19fbb0c02ccc6

sonarqubecloud · 2024-11-15T17:19:26Z

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Qup42 added 6 commits November 4, 2024 15:28

initial implementation of last frontend piece

f5b729d

actually call update

87ec543

reference latest prs in todos

32cb388

Merge branch 'master' into updateLastStep

d15e575

include missing header

a05a64a

remove testing code

f297753

joka921 requested changes Nov 7, 2024

View reviewed changes

src/engine/Server.cpp Outdated Show resolved Hide resolved

src/engine/Server.cpp Outdated Show resolved Hide resolved

src/engine/Server.cpp Outdated Show resolved Hide resolved

src/engine/Server.cpp Outdated Show resolved Hide resolved

src/engine/Server.cpp Outdated Show resolved Hide resolved

Qup42 added 2 commits November 7, 2024 21:33

code review

95a5f65

fix

8b8d931

joka921 requested changes Nov 8, 2024

View reviewed changes

Hannah Bast added 2 commits November 8, 2024 19:13

Merge remote-tracking branch 'origin/master' into updateLastStep

8ab90eb

Use the DeltaTriplesManager in Server.cpp

cfa166e

TODO: This does not yet work for concurrent update requests (which need to be serialized via a lock). But good enough for playing around already.

Merge remote-tracking branch 'origin/master' into updateLastStep

888830e

hannahbast marked this pull request as ready for review November 10, 2024 18:50

hannahbast changed the title ~~Enable processing of Updates in Server~~ Use DeltaTriplesManager to process update request Nov 10, 2024

Qup42 added 2 commits November 11, 2024 14:47

add command to clear delta triples which reset any triples modified b…

fcf42ca

…y updates

code review

e0e130d

joka921 requested changes Nov 13, 2024

View reviewed changes

src/index/DeltaTriples.cpp Outdated Show resolved Hide resolved

src/engine/Server.cpp Outdated Show resolved Hide resolved

src/engine/Server.cpp Outdated Show resolved Hide resolved

src/engine/Server.cpp Show resolved Hide resolved

code review

1ab5cc5

joka921 requested changes Nov 14, 2024

View reviewed changes

src/engine/Server.cpp Show resolved Hide resolved

src/engine/Server.cpp Outdated Show resolved Hide resolved

Qup42 and others added 5 commits November 14, 2024 16:54

code review

2155e1d

update 2 strings

f2f0bd5

Debug the failure on Conan.

e3e70e7

Another try of figuring out what is wrong with the conan build.

f5eb9d5

Signed-off-by: Johannes Kalmbach <[email protected]>

Don't directly co_await the coroutines in the conan build..

42f3846

Signed-off-by: Johannes Kalmbach <[email protected]>

joka921 approved these changes Nov 18, 2024

View reviewed changes

joka921 merged commit 97d5037 into ad-freiburg:master Nov 18, 2024
20 of 22 checks passed

Qup42 deleted the updateLastStep branch November 19, 2024 16:36

Qup42 mentioned this pull request Dec 12, 2024

Return metadata on executed Update #1675

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `DeltaTriplesManager` to process `update` request #1608

Use `DeltaTriplesManager` to process `update` request #1608

Qup42 commented Nov 7, 2024 •

edited by joka921

Loading

joka921 left a comment

joka921 left a comment

joka921 Nov 8, 2024

Qup42 Nov 13, 2024

hannahbast commented Nov 9, 2024

codecov bot commented Nov 10, 2024 •

edited

Loading

Qup42 commented Nov 11, 2024

joka921 left a comment

joka921 left a comment

sparql-conformance bot commented Nov 15, 2024

sonarqubecloud bot commented Nov 15, 2024

Use DeltaTriplesManager to process update request #1608

Use DeltaTriplesManager to process update request #1608

Conversation

Qup42 commented Nov 7, 2024 • edited by joka921 Loading

joka921 left a comment

Choose a reason for hiding this comment

joka921 left a comment

Choose a reason for hiding this comment

joka921 Nov 8, 2024

Choose a reason for hiding this comment

Qup42 Nov 13, 2024

Choose a reason for hiding this comment

hannahbast commented Nov 9, 2024

codecov bot commented Nov 10, 2024 • edited Loading

Codecov Report

Qup42 commented Nov 11, 2024

joka921 left a comment

Choose a reason for hiding this comment

joka921 left a comment

Choose a reason for hiding this comment

sparql-conformance bot commented Nov 15, 2024

Conformance check passed ✅

sonarqubecloud bot commented Nov 15, 2024

Quality Gate passed

Use `DeltaTriplesManager` to process `update` request #1608

Use `DeltaTriplesManager` to process `update` request #1608

Qup42 commented Nov 7, 2024 •

edited by joka921

Loading

codecov bot commented Nov 10, 2024 •

edited

Loading