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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-5231: fix poor query performance for hasStatements() in FedX #5232

Merged
merged 2 commits into from
Jan 24, 2025
Merged

GH-5231: fix poor query performance for hasStatements() in FedX #5232

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
29097d1
GH-5231: fix poor query performance for hasStatements() in FedX
aschwarte10 Jan 15, 2025
1cc4ab8
GH-5231: refine javadoc, add Experimental annotation
aschwarte10 Jan 23, 2025
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
21 changes: 21 additions & 0 deletions tools/federation/src/main/java/org/eclipse/rdf4j/federated/FedXConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,27 @@ protected SailException convert(RuntimeException e) {
}
}

@Override
protected boolean hasStatementInternal(Resource subj, IRI pred, Value obj, boolean includeInferred,
Resource[] contexts) {
try {
Dataset dataset = new SimpleDataset();
FederationEvalStrategy strategy = federationContext.createStrategy(dataset);
QueryInfo queryInfo = new QueryInfo(subj, pred, obj, 0, includeInferred, federationContext, strategy,
dataset);
federationContext.getMonitoringService().monitorQuery(queryInfo);
return strategy.hasStatements(queryInfo, subj, pred, obj, contexts);

} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
if (e instanceof InterruptedException) {
Thread.currentThread().interrupt();
}
throw new SailException(e);
}
}

@Override
protected void addStatementInternal(Resource subj, IRI pred, Value obj, Resource... contexts) throws SailException {
try {
Expand Down
43 changes: 42 additions & 1 deletion ...deration/src/main/java/org/eclipse/rdf4j/federated/evaluation/FederationEvalStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ public CloseableIteration<Statement> getStatements(QueryInfo queryInfo, Resource
IRI pred, Value obj, Resource... contexts)
throws RepositoryException, MalformedQueryException, QueryEvaluationException {

List<Endpoint> members = federationContext.getFederation().getMembers();
List<Endpoint> members = getAccessibleFederationMembers(queryInfo);

// a bound query: if at least one fed member provides results
// return the statement, otherwise empty result
Expand Down Expand Up @@ -605,6 +605,47 @@ public CloseableIteration<Statement> getStatements(QueryInfo queryInfo, Resource
return union;
}

/**
* Returns true if the federation has statements
*
* @param queryInfo information about the query
* @param subj the subject or <code>null</code>
* @param pred the predicate or <code>null</code>
* @param obj the object or <code>null</code>
* @param contexts optional list of contexts
* @return the statement iteration
*
* @throws RepositoryException
* @throws MalformedQueryException
* @throws QueryEvaluationException
*/
public boolean hasStatements(QueryInfo queryInfo, Resource subj,
IRI pred, Value obj, Resource... contexts)
throws RepositoryException, MalformedQueryException, QueryEvaluationException {

List<Endpoint> members = getAccessibleFederationMembers(queryInfo);

// form the union of results from relevant endpoints
List<StatementSource> sources = CacheUtils.checkCacheForStatementSourcesUpdateCache(cache, members, subj, pred,
obj, queryInfo, contexts);

if (sources.isEmpty()) {
return false;
}

return true;
}

/**
* Returns the accessible federation members in the context of the query. By default this is all federation members.
*
* @param queryInfo
* @return
*/
protected List<Endpoint> getAccessibleFederationMembers(QueryInfo queryInfo) {
return federationContext.getFederation().getMembers();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain more why this needs to be protected. I didn't see that it's override anywhere in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. We make use of specializations of FederationEvalStrategy to validate new optimizations (before contributing them back to RDF4J) or add additional (special) ones applicable to our use-cases. Without having this method accessible in sub-classes, we would need to override the two methods entirely (get/hasStatementsInternal) as we have a specialization for accessible members, currently validating resilience, but looking ahead also for policies/permissions. Does this help?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sound good. Can you update the docs to reflect that this is meant to be an extension point for subclasses. If you are unsure if you need to change it in future you can annotate it as experimental if you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, done


public CloseableIteration<BindingSet> evaluateService(FedXService service,
BindingSet bindings) throws QueryEvaluationException {

Expand Down
55 changes: 55 additions & 0 deletions tools/federation/src/test/java/org/eclipse/rdf4j/federated/BasicTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,61 @@ public void testBindClause() throws Exception {
execute("/tests/basic/query_bind.rq", "/tests/basic/query_bind.srx", false, true);
}

@Test
public void testRepositoryConnectionApi() throws Exception {

prepareTest(
Arrays.asList("/tests/basic/data_emptyStore.ttl", "/tests/basic/data_emptyStore.ttl"));

Repository repo1 = getRepository(1);
Repository repo2 = getRepository(2);

IRI bob = Values.iri("http://example.org/bob");
IRI alice = Values.iri("http://example.org/alice");
IRI graph1 = Values.iri("http://example.org/graph1");
IRI graph2 = Values.iri("http://example.org/graph2");

try (RepositoryConnection conn = repo1.getConnection()) {
conn.add(bob, RDF.TYPE, FOAF.PERSON, graph1);
conn.add(bob, FOAF.NAME, Values.literal("Bob"), graph1);
}

try (RepositoryConnection conn = repo2.getConnection()) {
conn.add(alice, RDF.TYPE, FOAF.PERSON, graph2);
conn.add(alice, FOAF.NAME, Values.literal("Alice"), graph2);
}

var fedxRepo = fedxRule.getRepository();

try (var conn = fedxRepo.getConnection()) {

// hasStatement which exist
Assertions.assertTrue(conn.hasStatement(bob, RDF.TYPE, FOAF.PERSON, false));
Assertions.assertTrue(conn.hasStatement(bob, RDF.TYPE, FOAF.PERSON, false, graph1));
Assertions.assertTrue(conn.hasStatement(null, RDF.TYPE, FOAF.PERSON, false));
Assertions.assertTrue(conn.hasStatement(null, RDF.TYPE, FOAF.PERSON, false, graph1));
Assertions.assertTrue(conn.hasStatement(null, RDF.TYPE, null, false));
Assertions.assertTrue(conn.hasStatement(null, RDF.TYPE, null, false, graph1));
Assertions.assertTrue(conn.hasStatement(null, RDF.TYPE, null, false, graph2));
Assertions.assertTrue(conn.hasStatement(null, null, null, false));
Assertions.assertTrue(conn.hasStatement(null, null, null, false, graph1));

// hasStatement which do not exist
Assertions.assertFalse(conn.hasStatement(bob, RDF.TYPE, FOAF.ORGANIZATION, false));
Assertions.assertFalse(conn.hasStatement(bob, RDF.TYPE, FOAF.PERSON, false, graph2));

// getStatements
Assertions.assertEquals(Set.of(bob, alice),
QueryResults.asModel(conn.getStatements(null, RDF.TYPE, FOAF.PERSON, false)).subjects());
Assertions.assertEquals(Set.of(bob),
QueryResults.asModel(conn.getStatements(null, RDF.TYPE, FOAF.PERSON, false, graph1)).subjects());
Assertions.assertEquals(Set.of(bob, alice),
QueryResults.asModel(conn.getStatements(null, null, null, false)).subjects());
Assertions.assertEquals(Set.of(bob),
QueryResults.asModel(conn.getStatements(null, null, null, false, graph1)).subjects());
}
}

@Test
public void testFederationSubSetQuery() throws Exception {
String ns1 = "http://namespace1.org/";
Expand Down
Loading