Tricium: Failed to call Launcher.Launch ... missing paths to analyze |
||||
Issue description
This is a recurring error in the tricium-dev logs:
Failed to call Launcher.Launch :: {"error":"rpc error: code = InvalidArgument desc = missing paths to analyze"
Example cases:
Run ID 5710606233501696
Run ID 6234694920175616
Run ID 5471866718257152
For some cases, I can't find the corresponding gerrit change.
For others, e.g. run 6234694920175616, it appears to map to an issue like https://gerrit-review.googlesource.com/c/gerrit/+/152792 that should have paths to analyze.
I believe I saw this error in logs for an instance deployed more than a week ago.
The error is logged here:
https://cs.chromium.org/chromium/infra/go/src/infra/tricium/appengine/launcher/rpc_launch.go?l=47
The launch request is created in the analyze request:
https://cs.chromium.org/chromium/infra/go/src/infra/tricium/appengine/frontend/rpc_analyze.go?l=165
,
Jan 25 2018
Note, there are tasks in the launcher-queue that are retried periodically that hit this error.
,
Mar 1 2018
,
Mar 1 2018
Note, examining one of those changes above, https://gerrit-review.googlesource.com/c/gerrit/+/152792/1, reveals that in some cases, there *really are* no paths to analyze in a CL. And of course, now that we're filtering out deleted files, it's certainly possible that there will be only deleted files. Of course, the commit message is there, and we might want to treat that as a file. It can't be checked out exactly the same as other files in GitFileIsolator, but we could treat it as a special case there. Possible solutions: - If there are no files, don't launch analysis. - If there are no files, launch analysis and then finish shortly afterwards with no results. - Count "Commit message" as a file. This potentially has the advantage that we could then put robocomments in the commit message (e.g. for spelling errors, whitespace issues, invalid commit footers). Then we could assume there's always one file.
,
Mar 2 2018
I'm not sure what to do yet, but for now I have a proposal: 1. First, launch analysis even if there are no paths. 2. Later, if we want to treat commit message as a regular file: a. Add support for the "/COMMIT_MSG" magic path in GitFileIsolator b. Add "/COMMIT_MSG" for all analyze requests for Gerrit changes.
,
Mar 2 2018
Made an initial CL https://chromium-review.googlesource.com/c/infra/infra/+/945008 to allow empty path list. But upon further consideration, I think it makes sense to avoid launching any workflow if there's definitely no work to do. If we do want to analyze the commit message, then we'll probably want to do it by adding /COMMIT_MSG in the list of paths in the AnalyzeRequest, in which case the list of paths won't be empty. Since it's still possible for the list of paths to be legitimately empty in any incoming AnalyzeRequest, I think it'd make sense to: 1 Allow an AnalysisRequest with empty paths, but just log and abort without launching a workflow run. 2 Don't allow a LaunchRequest with empty paths.
,
Mar 8 2018
Another thought: If we do want to analyze the commit message, then we may want to do it in gerrit/poll.go, because /COMMIT_MSG will be avilable for Gerrit changes, but not necessarily for all repos/refs that could be put in an AnalyzeRequest. In that case, if there are no Paths then there's nothing to analyze, so we could abort before creating an AnalyzeRequest. Proposed CL for this: https://canary-chromium-review.googlesource.com/c/infra/infra/+/956606
,
Mar 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/7ac76e75866a349747d441b3f24cde3f2330299f commit 7ac76e75866a349747d441b3f24cde3f2330299f Author: Quinten Yearsley <qyearsley@chromium.org> Date: Tue Mar 13 19:49:23 2018 Don't analyze changes with no files This change makes the Gerrit poller not make analyze requests for changes with no files. This is the case whenever the only file is /MERGE_LIST, or if the change only deletes files, etc. Related change https://crrev.com/c/950241 also adds validation for empty analyze requests added by the poller, which shouldn't happen any more after this CL. Bug: 803971 Change-Id: Ic94255441f879c994b1a2ad3ffb5c793f28d347c Reviewed-on: https://chromium-review.googlesource.com/956606 Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org> Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org> [modify] https://crrev.com/7ac76e75866a349747d441b3f24cde3f2330299f/go/src/infra/tricium/appengine/gerrit/poll.go [modify] https://crrev.com/7ac76e75866a349747d441b3f24cde3f2330299f/go/src/infra/tricium/appengine/gerrit/poll_test.go
,
Mar 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/7a301aca42c3da44c195309c9ec4a0fa02d986ba commit 7a301aca42c3da44c195309c9ec4a0fa02d986ba Author: Quinten Yearsley <qyearsley@chromium.org> Date: Tue Mar 13 21:48:36 2018 Validate AnalyzeRequests coming from the analyze queue. AnalyzeRequests going to the RPC endpoint are checked; they should be checked in the same way if they come through the analyze queue. The expected result of this is that CLs with only deleted files would now cause an error to be logged in the analyze queue handler. Related change: https://crrev.com/c/956606 Don't create analyze requests for changes with no paths. Bug: 803971 Change-Id: I711d756ca4d220230305e5cd6e7f898526bf2619 Reviewed-on: https://chromium-review.googlesource.com/950241 Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org> Commit-Queue: Quinten Yearsley <qyearsley@chromium.org> [modify] https://crrev.com/7a301aca42c3da44c195309c9ec4a0fa02d986ba/go/src/infra/tricium/appengine/frontend/handlers.go [modify] https://crrev.com/7a301aca42c3da44c195309c9ec4a0fa02d986ba/go/src/infra/tricium/appengine/frontend/handlers_test.go [modify] https://crrev.com/7a301aca42c3da44c195309c9ec4a0fa02d986ba/go/src/infra/tricium/appengine/common/testing/testing.go [modify] https://crrev.com/7a301aca42c3da44c195309c9ec4a0fa02d986ba/go/src/infra/tricium/appengine/frontend/run_test.go
,
Mar 19 2018
In theory this should now be fixed in the latest version. |
||||
►
Sign in to add a comment |
||||
Comment 1 by emso@chromium.org
, Jan 22 2018