[XL] Change in asterixdb[master]: [ASTERIXDB-3593]: Execute and report each statement of a request

[AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu>]

From Janhavi Tripurwar <ja...@apache.org>:

Attention is currently required from: Janhavi Tripurwar.

Hello Ali Alsuliman, Anon. E. Moose #1000171, Ian Maxon, Jenkins, Michael Blow, Murtadha Hubail, 

I'd like you to reexamine a change. Please visit

    https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19687?usp=email

to look at the new patch set (#47).

The following approvals got outdated and were removed:
Code-Review+1 by Janhavi Tripurwar, Integration-Tests+1 by Jenkins, Verified+1 by Jenkins


Change subject: [ASTERIXDB-3593]: Execute and report each statement of a request
......................................................................

[ASTERIXDB-3593]: Execute and report each statement of a request

- user model changes: no
- storage format changes: no
- interface changes: yes

Details:
- The statements of a request were executed one after another into one
  shared response: the results, status and metrics described all of
  them together, the stats and the statement properties of the request
  were overwritten by each statement in turn, and a failure could not
  be attributed to the statement that caused it.
- A request an NC forwarded is now reported one statement at a time:
  the CC prints each statement into an output of its own and hands the
  NC that output together with the statement's result metadata, stats,
  plans, warnings and error. That is how a forwarded request has always
  been served - the CC prints, the NC serves what it printed plus the
  result metadata - only per statement now. Statement execution is
  otherwise untouched: the statements print exactly what they printed
  before, and nothing chooses between printing and reporting.
- Only an NC-served request is reported per statement. A request a CC
  received itself prints its own response as it executes, exactly as
  before this change, and gets no per-statement responses.
- The response of AsterixDB's endpoints is unchanged, for any number
  of statements. What an endpoint does with the per-statement reports
  is its own decision: reporting them individually, as a "statements"
  array with one entry per statement, is implemented and available
  behind QueryServiceServlet.isStatementsEnvelopeSupported(), which
  answers false everywhere here, so turning it on is the only change
  an endpoint needs to adopt it.
- The multi-statement parameter keeps its meaning: whether a request
  may carry more than one statement.
- A response that holds the results of several statements still names
  them results, results-0, results-1, ...; only a response that reports
  each statement on its own names every statement's results results.
- A statement that fails stops the request: the statements before it
  keep the reports they already produced, and the ones after it do not
  execute. Everything that can fail for a single statement - its
  validation, its rewriting, its execution - is now attributed to that
  statement rather than to the request.
- Async delivery answers with a handle before the statement has run,
  so the statements of a request cannot be sequenced: async with more
  than one real statement is rejected (ASX1255). Deferred delivery also
  answers with a handle but runs the statements in order, so it stays
  supported, with a handle per statement.
- max-warnings is a per-statement budget. An endpoint that reports the
  request as a whole caps the aggregate at that budget, as the request
  did before.
- Parser warnings are attributed to the statement they were raised
  for, so IParser.parse() returns each statement with its own
  warnings. An unclaimed hint is reported once, against its own
  statement, instead of against every statement that follows it.
- New extension point validateStatement(), for validation that depends
  on the statement and on who is running it, as opposed to
  validateStatements(), which validates the request before any of its
  statements runs.
- A request whose statements produce no output of their own - a lone
  USE, SET or DECLARE FUNCTION - reports the status and metrics of the
  request, rather than a response with neither.
- ResultStatus and the request's timing and status move out of
  AbstractQueryApiServlet into IResultStatus, RequestExecutionState and
  RequestExecutionStatus, so the translator can report a statement's
  outcome without depending on the servlets. A request that fails
  during compilation or execution reports its real executionTime
  again, instead of 0.
- The printer forgets what it has printed between statements, so a
  statement's output opens with a field of its own rather than with the
  separator that would follow the statement before it, and each
  statement names its results results rather than results-0.
- QueryStatusApiServlet no longer prints the errors field twice when
  the status of a failed async job is polled.

Also in this change (chunked HTTP responses), since a failure after
part of a response was already flushed is now routine:
- An error raised after the response header was written closed the
  connection abruptly, so the client saw a truncated response with no
  error. It is now sent as the last chunk of the response that was
  already started, and the channel is closed afterwards; where it
  cannot be sent, the channel is still closed.
- beforeFlush() queued the shared response object with a void promise,
  so a servlet failing after the flush could change its status before
  the encoder ran, and the status line reaching the client depended on
  timing. It now queues a snapshot of the header.

Tests:
- MultiStatementResponseTest pins the response of a request carrying
  two statements, on a CC and on an NC, the status and metrics of a
  request whose only statement produces no output, and a handle per
  statement for deferred delivery. It is a JUnit test because the
  runtimets framework cannot read such a response: a flat response
  carrying two queries repeats the signature field, which its JSON
  reader rejects as a duplicate key.
- api/multi-statement covers the two rejections: several statements
  when the request does not allow them, and async with several
  statements.
- ResponsePrinterTest pins that the statements array is well-formed:
  its braces and separators are printer state, written as the
  statements execute, so closing an entry is idempotent (the loop and
  the request both close the entry they may own), an error reported
  without an open entry gets an entry of its own, a statement's output
  does not open with a separator, and each statement names its results
  results.
- HttpServerTest/ErrorAfterHeaderServlet cover the error raised after
  the response header was written.

Ext-ref: MB-62708

Change-Id: Iecba6ed2a30f7a2e7d8ff21e995117ed505dfad5
---
M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/ExecutionPlans.java
M asterixdb/asterix-algebra/src/main/java/org/apache/asterix/translator/IStatementExecutor.java
M asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/NCQueryServiceServlet.java
M asterixdb/asterix-app/src/main/java/org/apache/asterix/app/message/ExecuteStatementRequestMessage.java
A asterixdb/asterix-app/src/main/java/org/apache/asterix/app/result/fields/NcStatementsPrinter.java
M asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java
A asterixdb/asterix-app/src/test/java/org/apache/asterix/app/result/NcStatementsPrinterTest.java
M asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/ResultExtractor.java
A asterixdb/asterix-app/src/test/java/org/apache/asterix/test/runtime/MultiStatementResponseTest.java
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/multi-statement/multi-statement.1.post.http
A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/api/multi-statement/multi-statement.2.post.http
M asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/async-deferred/AsyncDeferredQueries.xml
M asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/async-deferred/async-exhausted-result/async-exhausted-result.4.get.uri
A asterixdb/asterix-app/src/test/resources/runtimets/results/async-deferred/async-exhausted-result/async-exhausted-result.4.ignore
M asterixdb/asterix-app/src/test/resources/runtimets/sqlpp_queries.xml
M asterixdb/asterix-common/src/main/java/org/apache/asterix/common/exceptions/ErrorCode.java
M asterixdb/asterix-common/src/main/resources/asx_errormsg/en.properties
M asterixdb/asterix-lang-common/src/main/java/org/apache/asterix/lang/common/base/IParser.java
M asterixdb/asterix-lang-sqlpp/src/main/javacc/SQLPP.jj
M hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/ChunkedResponse.java
M hyracks-fullstack/hyracks/hyracks-http/src/test/java/org/apache/hyracks/test/http/HttpServerTest.java
A hyracks-fullstack/hyracks/hyracks-http/src/test/java/org/apache/hyracks/test/http/servlet/ErrorAfterHeaderServlet.java
M hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/annotations/AiProvenance.java
23 files changed, 1,529 insertions(+), 225 deletions(-)


  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/87/19687/47
-- 
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19687?usp=email
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Change-Id: Iecba6ed2a30f7a2e7d8ff21e995117ed505dfad5
Gerrit-Change-Number: 19687
Gerrit-PatchSet: 47
Gerrit-Owner: Janhavi Tripurwar <ja...@apache.org>
Gerrit-Reviewer: Ali Alsuliman <al...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Ian Maxon <im...@apache.org>
Gerrit-Reviewer: Janhavi Tripurwar <ja...@apache.org>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-Reviewer: Murtadha Hubail <mh...@apache.org>
Gerrit-Attention: Janhavi Tripurwar <ja...@apache.org>