RickMoynihan
commented
6 years ago Interesting, this is on the "pull" interface; but does it happen on the push interface? I seem to recall there being code that handled things there differently.
Generally speaking there is a fundamental problem here, which is that many of the RDF serialization formats don't let you know when you have the complete document. i.e. if when parsing n-triples, n-quads, or turtle (and probably some other formats) the underlying stream is closed early a client can't tell whether it has the complete data or if the connection was just closed early, which just happened to happen at a valid end point, e.g.
<http://foo> <http://foo> <http://foo> .
<http://bar> <http://bar> <http://bar> . |CONNECTION DIES|The client can't tell that there should have been more triples.
Some formats are more robust against this, i.e. JSON/XML based ones will have a closing tag which should only be sent when the server has actually finished. Though we can't use XML for constructs because it can't actually represent all valid RDF graphs.
We might be able to resolve some of this issue by configuring these whitelists for content neg. However the issue may remain somewhat inside the pull interface because it knows nothing of the underlying format. IIRC the pull interface should make a
We can also experience this undesired behaviour in non-exceptional cases, such as any time we close the stream before fully consuming all of the results. I'm not sure if we do do this, but it's something worth considering in the overall fix.
FYI Drafter should always eagerly consume the query results it makes internally, as evidenced by the stack frame at drafter.rdf.sparql$eager_query.invokeStatic(sparql.clj:21), so it should be safe to make this assumption.
Regardless of the above I think this is a problem, but there is no good solution available whilst we use the RDF4j pull interface to evaluate. Instead I think we should look at changing the grafter function:
https://github.com/swirrl/grafter/blob/rdf4j-temp/src/grafter/rdf4j/repository.clj#L386-L405
To instead call (.evaluate prepared-query handler); by passing a custom handler here we should then get a little finer granularity on the underlying events. Obviously if we were to do this we'd also need to preserve the lazy-seq return value interface, so we'd have to marshal the events back via a channel. We do this elsewhere in grafter with this pipe function:
https://github.com/swirrl/grafter/blob/rdf4j-temp/src/grafter/rdf4j/io.clj#L388-L400
I'd suggest we have a chat tomorrow about what exactly we want to do here, as it's a little involved.
danmidwood
Issue in investigation...
We see various problems reading state graph query results from the cache. It looks like we might be inserting incomplete cache entries, which then cause an exception at the point of reading.
This is logical, because we move the file from tmp to the cache on stream close, and we don't have any validation in there to ensure that the cache entry is complete.
In a case that we encounter a problem and call
stream.closewithout having processed the entire stream then we would still continue to insert the cache entry, even though we have not fully populated the file.We can also experience this undesired behaviour in non-exceptional cases, such as any time we close the stream before fully consuming all of the results. I'm not sure if we do do this, but it's something worth considering in the overall fix.
The exception we see reading from the cache:
Which goes into this code block and throw the exception on the
converter-fline