Fix flaky RdeStagingActionDatastoreTest (#1514)

* Fix flaky RdeStagingActionDatastoreTest

Fixed the most common cause that makes one method flaky (Clock and
timestamp problem). Added a TODO to rethink test case.

Also added notes on tasks potentially enqueued multiple times.
This commit is contained in:
Weimin Yu
2022-02-04 10:40:52 -05:00
committed by GitHub
parent 62e5de8a3a
commit b412bdef9f
4 changed files with 21 additions and 1 deletions
@@ -297,6 +297,10 @@ public class RdeIO {
logger.atInfo().log(
"Rolled forward %s on %s cursor to %s.", key.cursor(), key.tld(), newPosition);
RdeRevision.saveRevision(key.tld(), key.watermark(), key.mode(), revision);
// Enqueueing a task is a side effect that is not undone if the transaction rolls
// back. So this may result in multiple copies of the same task being processed.
// This is fine because the RdeUploadAction is guarded by a lock and tracks progress
// by cursor. The BrdaCopyAction writes a file to GCS, which is an atomic action.
if (key.mode() == RdeMode.FULL) {
cloudTasksUtils.enqueue(
RDE_UPLOAD_QUEUE,
@@ -87,6 +87,8 @@ public final class BrdaCopyAction implements Runnable {
}
private void copyAsRyde() throws IOException {
// TODO(b/217772483): consider guarding this action with a lock and check if there is work.
// Not urgent since file writes on GCS are atomic.
int revision =
RdeRevision.getCurrentRevision(tld, watermark, THIN)
.orElseThrow(
@@ -226,6 +226,10 @@ public final class RdeStagingReducer extends Reducer<PendingDeposit, DepositFrag
logger.atInfo().log(
"Rolled forward %s on %s cursor to %s.", key.cursor(), tld, newPosition);
RdeRevision.saveRevision(tld, watermark, mode, revision);
// Enqueueing a task is a side effect that is not undone if the transaction rolls
// back. So this may result in multiple copies of the same task being processed. This
// is fine because the RdeUploadAction is guarded by a lock and tracks progress by
// cursor. The BrdaCopyAction writes a file to GCS, which is an atomic action.
if (mode == RdeMode.FULL) {
cloudTasksUtils.enqueue(
"rde-upload",