Issue 8230 - Unit test leaf partition query lambda, simplify closing iterator - #8231
patchwork01 wants to merge 16 commits into
Conversation
patchwork01
changed the title
patchwork01
added
the
needs-reviewer
| Map<String, String> resultsPublisherConfig = query.getProcessingConfig().getResultsPublisherConfig(); | ||
| ResultsOutputInfo outputInfo = getResultsOutput(tableProperties, resultsPublisherConfig) | ||
| .publish(query, results); | ||
| try (CloseableIterator<Row> results = getLeafPartitionQueryExecutor(tableProperties).getRows(leafPartitionQuery)) { |
There was a problem hiding this comment.
This can currently cause a query to initial get marked completed, then if the close fails the query gets reported as failed. Would it be worth trying to catch close errors and swallowing them like the old finally blocks did
There was a problem hiding this comment.
Good spot, thanks. It looks like the problem is the iterator isn't closed until after the query is marked as completed. It's also not reporting the ResultsOutputInfo to the tracker if there's a failure after it returns from the results output. I'll try and get this behaviour back to the way it was.
| return new NoResultsOutput(); | ||
| } else { | ||
| LOGGER.error("Unknown results publisher config: {}", resultsPublisherConfig); | ||
| throw new RuntimeException("Unknown results publisher for destination: " + destination); |
There was a problem hiding this comment.
Previously this section returned a ResultOutput carrying the error which I believe would result in a completed-with-error status, whereas now the exception throw will cause a failed state. I just wanted to check that change was intentional?
There was a problem hiding this comment.
It was, but I didn't realise it was actually recording as completed in the tracker. I think that seems like a bug though, it seems incorrect to report a completed status when the client's processing configuration wasn't applied and no results were published anywhere. I think we should keep this change.
ca61688
removed
the
needs-reviewer
2 participants
Make sure you have checked all steps below.
Issue
Feature". Note that before an issue is finished, you can still make a pull request by raising a separate issue
for your progress.
Tests
Documentation
separate issue for that below.