Skip to content

Commit 0167dad

Browse files
authored
Fix OOM error in BsaValidation (#2813)
Error happened in the case that an unblockable name reported with 'Registered' as reason has been deregistered. We tried to check the deletion time of the domain to decide if this is a transient error that is no worth reporting. However, we forgot that we do not have the domain key in this case. As best-effort action, and with a case that rarely happens, we decide not to make the optimization (staleness check) in thise case.
1 parent 1eaf3d4 commit 0167dad

File tree

2 files changed

+15
-33
lines changed

2 files changed

+15
-33
lines changed

core/src/main/java/google/registry/bsa/BsaValidateAction.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,12 @@ Optional<String> verifyDomainStillUnblockableWithReason(
215215
if (Objects.equals(expectedReason, domain.reason())) {
216216
return Optional.empty();
217217
}
218-
if (isRegistered || domain.reason().equals(Reason.REGISTERED)) {
219-
if (isStalenessAllowed(isRegistered, activeDomains.get(domain.domainName()))) {
218+
// Registered name still reported with other reasons: Don't report if registration is recent.
219+
// Note that staleness is not tolerated if deregistered name is still reported as registered:
220+
// in this case we do not have the VKey on hand, and it is not worth the effort to find it
221+
// out.
222+
if (isRegistered && isStalenessAllowed(activeDomains.get(domain.domainName()))) {
220223
return Optional.empty();
221-
}
222224
}
223225
return Optional.of(
224226
String.format(
@@ -228,15 +230,10 @@ Optional<String> verifyDomainStillUnblockableWithReason(
228230
domain.reason()));
229231
}
230232

231-
boolean isStalenessAllowed(boolean isNewDomain, VKey<Domain> domainVKey) {
233+
boolean isStalenessAllowed(VKey<Domain> domainVKey) {
232234
Domain domain = bsaQuery(() -> replicaTm().loadByKey(domainVKey));
233235
var now = clock.nowUtc();
234-
if (isNewDomain) {
235-
return domain.getCreationTime().plus(maxStaleness).isAfter(now);
236-
} else {
237-
return domain.getDeletionTime().isBefore(now)
238-
&& domain.getDeletionTime().plus(maxStaleness).isAfter(now);
239-
}
236+
return domain.getCreationTime().plus(maxStaleness).isAfter(now);
240237
}
241238

242239
/** Returns unique labels across all block lists in the download specified by {@code jobName}. */

core/src/test/java/google/registry/bsa/BsaValidateActionTest.java

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import static google.registry.bsa.persistence.BsaTestingUtils.persistUnblockableDomain;
2525
import static google.registry.testing.DatabaseHelper.createTld;
2626
import static google.registry.testing.DatabaseHelper.persistActiveDomain;
27-
import static google.registry.testing.DatabaseHelper.persistDeletedDomain;
2827
import static google.registry.testing.DatabaseHelper.persistResource;
2928
import static google.registry.util.DateTimeUtils.START_OF_TIME;
3029
import static org.mockito.ArgumentMatchers.any;
@@ -218,7 +217,8 @@ void checkBsaLabels_withErrors() throws Exception {
218217
test1,1;2
219218
test2,3
220219
""";
221-
String blockPlusContent = """
220+
String blockPlusContent =
221+
"""
222222
domainLabel,orderIDs
223223
test2,4
224224
""";
@@ -239,31 +239,15 @@ void isStalenessAllowed_newDomain_allowed() {
239239
persistBsaLabel("label");
240240
Domain domain = persistActiveDomain("label.app", fakeClock.nowUtc());
241241
fakeClock.advanceBy(MAX_STALENESS.minus(Duration.standardSeconds(1)));
242-
assertThat(action.isStalenessAllowed(true, domain.createVKey())).isTrue();
242+
assertThat(action.isStalenessAllowed(domain.createVKey())).isTrue();
243243
}
244244

245245
@Test
246246
void isStalenessAllowed_newDomain_notAllowed() {
247247
persistBsaLabel("label");
248248
Domain domain = persistActiveDomain("label.app", fakeClock.nowUtc());
249249
fakeClock.advanceBy(MAX_STALENESS);
250-
assertThat(action.isStalenessAllowed(true, domain.createVKey())).isFalse();
251-
}
252-
253-
@Test
254-
void isStalenessAllowed_deletedDomain_allowed() {
255-
persistBsaLabel("label");
256-
Domain domain = persistDeletedDomain("label.app", fakeClock.nowUtc());
257-
fakeClock.advanceBy(MAX_STALENESS.minus(Duration.standardSeconds(1)));
258-
assertThat(action.isStalenessAllowed(false, domain.createVKey())).isTrue();
259-
}
260-
261-
@Test
262-
void isStalenessAllowed_deletedDomain_notAllowed() {
263-
persistBsaLabel("label");
264-
Domain domain = persistDeletedDomain("label.app", fakeClock.nowUtc());
265-
fakeClock.advanceBy(MAX_STALENESS);
266-
assertThat(action.isStalenessAllowed(false, domain.createVKey())).isFalse();
250+
assertThat(action.isStalenessAllowed(domain.createVKey())).isFalse();
267251
}
268252

269253
@Test
@@ -405,10 +389,11 @@ void notificationSent_withValidationError() {
405389
assertThat(message.body())
406390
.isEqualTo(
407391
"""
408-
Most recent download is 2023-11-09t020857.880z.
392+
Most recent download is 2023-11-09t020857.880z.
409393
410-
Error line 1.
411-
Error line 2""");
394+
Error line 1.
395+
Error line 2\
396+
""");
412397
}
413398

414399
@Test

0 commit comments

Comments
 (0)