commit 227ea7a3b1ea37246fcb63eb7cbd9b69f67bf32c
author Tuan Nguyen <nqmtuan@google.com> Thu Jul 25 13:01:16 2024
committer LUCI CQ <infra-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jul 25 13:01:16 2024
tree f1bcd9a60e087e48b083204a3e76fbab093898c8
parent 56de5fedff641233c2978bd83a07089f7eac8d9d

Revert "Reland "Reland "[ArtifactExporter] Add checkpoints"""

This reverts commit 1a554ff3194b289bf7bfd03221f7c9e78e3ea94b.

Reason for revert: Still seeing elevated queue depth in dev.

Original change's description:
> Reland "Reland "[ArtifactExporter] Add checkpoints""
>
> This reverts commit cf1ed7dda520fbe49c0f4dc227f66adf4a5918f2.
>
> Relevant changes since last review: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/5732803/2..3
>
> Looking at the logs when the problem happenned [1] (link in comment),  I can see most of the time out is related to the download artifact (although it is also possible that the export artifact takes so long that it did not consume the channel fast enough, leading to the slowness of the download).
>
> I look through the code and see little relation between adding the checkpoint and the increasing of download time. So my suspect goes to the increasing of ChromeOSMaxArtifactSize from 5MB -> 50 MB. My hypothesis goes like this:
> - There was nothing wrong with the checkpoints or spanner.
> - When we increased the ChromeOSMaxArtifactSize to 50MB, some tasks takes longer and time out.
> - The increasing of ChromeOSMaxArtifactSize to 50MB should only affect the amount of artifact downloaded in streaming manner, but for some reason they also congest the batch manner download as well (?). I saw some tasks that tried to download only vary small amount of small artifacts [2] but timed out after 10 mins.
>
> I checked the RBE quota around that time, but there was no noticeable increase.
>
>
> Original change's description:
> > Revert "Reland "[ArtifactExporter] Add checkpoints""
> >
> > This reverts commit 0b5a5a622fe3f588bfafb824cd0e90fbcb5e1cd8.
> >
> > Reason for revert: I see an increase in queue size and tasks got deadline exceeded.
> >
> > Original change's description:
> > > Reland "[ArtifactExporter] Add checkpoints"
> > >
> > > This is a reland of commit 2e4251df7f1a1391d02873415e7edbb98c648871
> > >
> > > Changes relative to last review: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/5715471/1..3
> > >
> > > According to https://screenshot.googleplex.com/3gVQWA4sQjs4kK3, there were 2 common root cause of session leak. I checked for RowIterator and they all seem to be properly closed.
> > >
> > > One thing suspicious is that we created the ReadOnlyTransaction at the beginning, and keep that transaction through the whole task, which maybe 10 mins long. We only need it for a few queries at the beginning. This may contribute to the problem.
> > >
> > > Also added some logging and change from span.Apply -> BufferWrite (I am not sure if it helps).
> > >
> > > It this still does not solve the problem, my next step is to enable the TrackSessionHandles.
> > >
> > >
> > >
> > > Original change's description:
> > > > [ArtifactExporter] Add checkpoints
> > > >
> > > > In this CL:
> > > > - I increased the max artifact size for ChromeOS to 50MB, as we already have the limit for total invocation in place.
> > > > - We will increase the limit for invocation level later, after we verify checkpoint is working.
> > > > - Support checkpoint for artifacts.
> > > > - For big artifacts, we need special handling, because part of the artifact may be successfully exported in previous invocation run and we don't want to export them again.
> > > >
> > > > Bug:b/351046122
> > > > Change-Id: I0e64f809bc097a758481a20e91a4f762db15a6b9
> > > > Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/5686697
> > > > Reviewed-by: Patrick Meiring <meiring@google.com>
> > > > Commit-Queue: Tuan Nguyen <nqmtuan@google.com>
> > >
> > > Bug: b/351046122
> > > Change-Id: I0a6d6ffa79d4f8fbce205c1ad9466e7f42dc3016
> > > Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/5715471
> > > Commit-Queue: Tuan Nguyen <nqmtuan@google.com>
> > > Reviewed-by: Patrick Meiring <meiring@google.com>
> >
> > Bug: b/351046122
> > Change-Id: Ibdc7f9ee35f4c6d9b963d3d0edcdd3317570dc3f
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/5719828
> > Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> > Auto-Submit: Tuan Nguyen <nqmtuan@google.com>
> > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
>
> Bug: b/351046122
> Change-Id: Id428cb339985dee8cf812d3edf423304cef3bc3c
> Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/5732803
> Reviewed-by: Patrick Meiring <meiring@google.com>
> Commit-Queue: Tuan Nguyen <nqmtuan@google.com>

Bug: b/351046122
Change-Id: I4d6473292983e165c3f1e3e44266f79102c41ba6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/infra/luci/luci-go/+/5739946
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Tuan Nguyen <nqmtuan@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>