Make NordnUploadAction resilient to duplicate task queue tasks (#1516)

This is necessary because the Cloud Tasks API is not transactionally enrolled,
so it's possible that multiple tasks might end up being enqueued. We need to be
able to handle them.
This commit is contained in:
Ben McIlwain 2022-02-14 11:59:46 -08:00 committed by GitHub
parent 1af163d9ba
commit 94c33eba7e
2 changed files with 30 additions and 5 deletions

View file

@ -39,8 +39,11 @@ import com.google.appengine.api.urlfetch.HTTPRequest;
import com.google.appengine.api.urlfetch.HTTPResponse;
import com.google.appengine.api.urlfetch.URLFetchService;
import com.google.apphosting.api.DeadlineExceededException;
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Ordering;
import com.google.common.flogger.FluentLogger;
import google.registry.config.RegistryConfig.Config;
import google.registry.request.Action;
@ -125,15 +128,19 @@ public final class NordnUploadAction implements Runnable {
* delimited String.
*/
static String convertTasksToCsv(List<TaskHandle> tasks, DateTime now, String columns) {
String header = String.format("1,%s,%d\n%s\n", now, tasks.size(), columns);
StringBuilder csv = new StringBuilder(header);
// Use a Set for deduping purposes so we can be idempotent in case tasks happened to be
// enqueued multiple times for a given domain create.
ImmutableSortedSet.Builder<String> builder =
new ImmutableSortedSet.Builder<>(Ordering.natural());
for (TaskHandle task : checkNotNull(tasks)) {
String payload = new String(task.getPayload(), UTF_8);
if (!Strings.isNullOrEmpty(payload)) {
csv.append(payload).append("\n");
builder.add(payload + '\n');
}
}
return csv.toString();
ImmutableSortedSet<String> csvLines = builder.build();
String header = String.format("1,%s,%d\n%s\n", now, csvLines.size(), columns);
return header + Joiner.on("").join(csvLines);
}
/** Leases and returns all tasks from the queue with the specified tag tld, in batches. */
@ -168,6 +175,11 @@ public final class NordnUploadAction implements Runnable {
: LordnTaskUtils.QUEUE_CLAIMS);
String columns = phase.equals(PARAM_LORDN_PHASE_SUNRISE) ? COLUMNS_SUNRISE : COLUMNS_CLAIMS;
List<TaskHandle> tasks = loadAllTasks(queue, tld);
// Note: This upload/task deletion isn't done atomically (it's not clear how one would do so
// anyway). As a result, it is possible that the upload might succeed yet the deletion of
// enqueued tasks might fail. If so, this would result in the same lines being uploaded to NORDN
// across mulitple uploads. This is probably OK; all that we really cannot have is a missing
// line.
if (!tasks.isEmpty()) {
String csvData = convertTasksToCsv(tasks, now, columns);
uploadCsvToLordn(String.format("/LORDN/%s/%s", tld, phase), csvData);

View file

@ -138,13 +138,26 @@ class NordnUploadActionTest {
void test_convertTasksToCsv() {
List<TaskHandle> tasks =
ImmutableList.of(
makeTaskHandle("task1", "example", "csvLine1", "lordn-sunrise"),
makeTaskHandle("task2", "example", "csvLine2", "lordn-sunrise"),
makeTaskHandle("task1", "example", "csvLine1", "lordn-sunrise"),
makeTaskHandle("task3", "example", "ending", "lordn-sunrise"));
assertThat(NordnUploadAction.convertTasksToCsv(tasks, clock.nowUtc(), "col1,col2"))
.isEqualTo("1,2010-05-01T10:11:12.000Z,3\ncol1,col2\ncsvLine1\ncsvLine2\nending\n");
}
@MockitoSettings(strictness = Strictness.LENIENT)
@Test
void test_convertTasksToCsv_dedupesDuplicates() {
List<TaskHandle> tasks =
ImmutableList.of(
makeTaskHandle("task2", "example", "csvLine2", "lordn-sunrise"),
makeTaskHandle("task1", "example", "csvLine1", "lordn-sunrise"),
makeTaskHandle("task3", "example", "ending", "lordn-sunrise"),
makeTaskHandle("task1", "example", "csvLine1", "lordn-sunrise"));
assertThat(NordnUploadAction.convertTasksToCsv(tasks, clock.nowUtc(), "col1,col2"))
.isEqualTo("1,2010-05-01T10:11:12.000Z,3\ncol1,col2\ncsvLine1\ncsvLine2\nending\n");
}
@MockitoSettings(strictness = Strictness.LENIENT)
@Test
void test_convertTasksToCsv_doesntFailOnEmptyTasks() {