Commit graph

1555 commits

Author SHA1 Message Date
Weimin Yu
1a4a31569e
Alt entity model for fast JPA bulk query (#1398)
* Alt entity model for fast JPA bulk query

Defined an alternative JPA entity model that allows fast bulk loading of
multi-level entities, DomainBase and DomainHistory. The idea is to bulk
the base table as well as the child tables separately, and assemble them
into the target entity in memory in a pipeline.

For DomainBase:

- Defined a DomainBaseLite class that models the "Domain" table only.

- Defined a DomainHost class that models the "DomainHost" table
  (nsHosts field).

- Exposed ID fields in GracePeriod so that they can be mapped to domains
  after being loaded into memory.

For DomainHistory:

- Defined a DomainHistoryLite class that models the "DomainHistory"
  table only.

- Defined a DomainHistoryHost class that models its namesake table.

- Exposed ID fields in GracePeriodHistory and DomainDsDataHistory
  classes so that they can be mapped to DomainHistory after being
  loaded into memory.

In PersistenceModule, provisioned a JpaTransactionManager that uses
the alternative entity model.

Also added a pipeline option that specifies which JpaTransactionManager
to use in a pipeline.
2021-10-20 16:48:56 -04:00
gbrodman
c7f50dae92
Use READ_COMMITTED serialization level in CreateSyntheticHEA (#1395)
I observed an instance in which a couple queries from this action were,
for whatever reason, hanging around as idle for >30 minutes. Assuming
the behavior that we saw before where "an open idle serializable
transaction means all pg read-locks stick around forever" still holds,
that's the reason why the amount of read-locks in use spirals out of
control.

I'm not sure why those queries aren't timing out, but that's a separate
issue.
2021-10-19 11:36:15 -04:00
gbrodman
969fa2b68c
Fix weird flake (#1394) 2021-10-15 18:00:46 -04:00
gbrodman
9a569198fb
Ignore class visibility in EntityTest (#1389) 2021-10-15 17:08:51 -04:00
gbrodman
8a53edd57b
Use multiple transactions in IcannReportingUploadAction (#1386)
Relevant error log message: https://pantheon.corp.google.com/logs/viewer?project=domain-registry&minLogLevel=0&expandAll=false&timestamp=2021-10-11T15:28:01.047783000Z&customFacets=&limitCustomFacetWidth=true&dateRangeEnd=2021-10-11T20:51:40.591Z&interval=PT1H&resource=gae_app&logName=projects%2Fdomain-registry%2Flogs%2Fappengine.googleapis.com%252Frequest_log&scrollTimestamp=2021-10-11T15:10:23.174336000Z&filters=text:icannReportingUpload&dateRangeUnbound=backwardInTime&advancedFilter=resource.type%3D%22gae_app%22%0AlogName%3D%22projects%2Fdomain-registry%2Flogs%2Fappengine.googleapis.com%252Frequest_log%22%0A%22icannReportingUpload%22%0Aoperation.id%3D%22616453df00ff02a873d26cedb40001737e646f6d61696e2d726567697374727900016261636b656e643a6e6f6d756c75732d76303233000100%22

note the "invalid handle" bit

From https://cloud.google.com/datastore/docs/concepts/transactions:
"Transactions expire after 270 seconds or if idle for 60 seconds."

From b/202309933: "There is a 60 second timeout on Datastore operations
after which they will automatically rollback and the handles become
invalid."

From the logs we can see that the action is lasting significantly longer
than 270 seconds -- roughly 480 seconds in the linked log (more or
less). My running theory is that ICANN is, for some reason, now being
significantly more slow to respond than they used to be. Some uploads in
the log linked above are taking upwards of 10 seconds, especially when
they have to retry. Because we have >=45 TLDs, it's not surprising that
the action is taking >400 seconds to run.

The fix here is to perform each per-TLD operation in its own
transaction. The only reason why we need the transactions is for the
cursors anyway, and we can just grab and store those at the beginning of
the transaction.
2021-10-15 15:38:37 -04:00
Lai Jiang
d25d4073f5
Add a beam pipeline to create synthetic history entries in SQL (#1383)
* Add a beam pipeline to create synthetic history entries in SQL

The logic is mostly lifted from CreateSyntheticHistoryEntriesAction. We
do not need to test for the existence of an embedded EPP resource in the
history entry before create a synthetic one because after
InitSqlPipeline runs it is guaranteed that no embedded resource exists.
2021-10-15 14:51:01 -04:00
Ben McIlwain
6ffe84e93d
Add a scrap command to hard-delete a host resource (#1391) 2021-10-15 12:28:18 -04:00
Rachel Guan
bb8988ee4e
Set payload in success response after sending notification emails (#1377)
* Set payload in success response after sending expiring certificate notification emails

* Modify log message and test cases for run() in sendExpiringCertificateNotificationEmailAction
2021-10-13 15:58:25 -04:00
Rachel Guan
2aff72b3b6
Add reason and requestedByRegistrar to domain renew flow (#1378)
* Resolve merge conflict

* Include reason and requestedByRegistrar in URS test file

* Modify test cases for new parameters in renew flow

* Add reason and registrar_request to renew domain command

* Update comments for new params in renew flow

* Make changes based on feedback
2021-10-13 11:41:02 -04:00
Weimin Yu
35fd61f771
Update parameter to Datastore wipe pipeline (#1385)
* Update parameter to Datastore wipe pipeline

Add the newly required RegistryEnvironment parameter to
BulkDeleteDatastorePipeline.

Remove the nullable annotation for this parameter in options
class.

Update metadata files regarding this parameter.
2021-10-11 17:31:50 -04:00
Michael Muller
13cb17e9a4
Implement several fixes affecting test flakiness (#1379)
* Implement several fixes affecting test flakiness

- Continued to do transaction manager cleanups on reply failure (lack of this
  may be causing cascading failures.
- Fix UpdateDomainCommandTest's output check (the test was checking for error
  output in standard error, but the command writes its output to the logs.
  Apparently, these may or may not be reflected in standard error depending on
  current global state)
- Remove unnecessary locking and incorrect comment in CommandTestCase.  The
  JUnit tests are not run in parallel in the same JVM and, in general, there
  are much bigger obstacles to this than standard output stream locking.

* Fix bad log message check
2021-10-11 12:54:03 -04:00
Ben McIlwain
4f1c317bbc
Revert update auto timestamp non-transactional fallback (#1380)
This was added recently in PR #1341 as an attempted fix for our test flakiness,
but it turns out that it didn't address the root issue (whereas PR #1361
did). So this removes the fallback, as there's no reason this should ever be
called outside of a transactional context.
2021-10-08 16:44:45 -04:00
gbrodman
c8aa32ef05
Include more info in host/domain name failures (#1346)
We're seeing some of these in CreateSyntheticHistoryEntriesAction and I
can't tell why from the logs (it doesn't appear to print the repo ID or
domain/host name)
2021-10-08 15:17:22 -04:00
gbrodman
95a1bbf66a
Temporarily disable SQL->DS replay in all tests (#1363) 2021-10-08 14:15:57 -04:00
Rachel Guan
23aa16469e
Add WipeOutContactHistoryPiiAction to prod (#1356) 2021-10-08 11:46:26 -04:00
Ben McIlwain
0277c5c25a
Add TmOverrideExtension for more safe TM overrides in tests (#1382)
* Add TmOverrideExtension for more safe TM overrides in tests

This is safer to use than calling setTmForTest() directly because this extension
also handles the corresponding call to removeTmOverrideForTest() automatically,
the forgetting of which has been a source of test flakiness/instability in the
past.

There are now broadly two ways to get tests to run in JPA: either use
DualDatabaseTest, an AppEngineExtension, and the corresponding JPA-specific
@Test annotations, OR use this override alongside a
JpaTransactionManagerExtension.
2021-10-07 19:26:25 -04:00
Ben McIlwain
b1b0589281
Elaborate on database read-only error message (#1355)
* Elaborate on database read-only error message
2021-10-07 13:25:24 -04:00
Ben McIlwain
28628564cc
Set response payload when wiping out contact history PII (#1376)
Also uses smaller batches in tests so that they don't take so long.
2021-10-07 12:43:41 -04:00
Michael Muller
835f93f555
Add a reference to RDAP conformance checker (#1358)
* Add a reference to RDAP conformance checker

Make a note of the RDAP conformance checker for the next time that we deal
with the RDAP code - would be nice to have this in the test suite.

* Reformat comment
2021-10-07 12:34:41 -04:00
Ben McIlwain
276c188e9d
Canonicalize domain/host names in initial import script (#1347)
* Canonicalize domain/host names in initial import script

* Add tests and make reduce some method visibility
2021-10-07 11:59:46 -04:00
Rachel Guan
34ecc6fbe7
Add new parameter renew_one_year to URS (#1364)
* Add autorenews to URS (#1343)

* Add autorenews to URS

* Add autorenews to existing xml files for test cases

* Harmonize domain.get() in existing code

* Fix typo in test case name

* Modify existing test helper method to allow testing with different domain bases
2021-10-06 20:40:43 -04:00
gbrodman
0f4156c563
Use a more efficient query to find resources in histories (#1354) 2021-10-06 15:20:31 -04:00
Ben McIlwain
51b2887709
Fix BigQuery data set name handling in activity reporting (#1361)
* Fix BigQuery data set name handling in activity reporting

This is not a constant (as it depends on runtime state), so it can't be named
using UPPER_SNAKE_CASE. Additionally, it's not good practice to use field
initialization when there's logic depending on runtime state involved. So this
PR changes the class to use constructor injection and moves the logic into the
constructor.

* Add fix for ICANN reporting provide

* Extract out ICANN reporting data set

* Inject TransactionManager

* Make TransactionInfo static (per Mike)

* Use ofyTm() in BackupTestStore

* Revert extraneous formatting

* Use auditedOfy in CommitLogMutationTest
2021-10-05 15:11:03 -04:00
Lai Jiang
62eb8801c5
Finish RDE pipeline implementation in SQL mode (#1330)
This PR adds the final step in RDE pipeline (enqueueing the next action
  to Cloud Tasks) and makes some necessary changes, namely by making all
  CloudTasksUtils related classes serializable, so that they can be used
  on Beam.

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1330)
<!-- Reviewable:end -->
2021-10-04 21:02:44 -04:00
Lai Jiang
c6705d1956
Fix sandbox cron (#1366)
* Fix sandbox cron

"synchronized" can only be used to specify a 24h time range that is
evenly divided by the interval value, e. g. "every 2 hours
synchronized".

* Change to a different time
2021-10-04 11:09:55 -04:00
Lai Jiang
c8caa8f80b
Remove the use of AppEngineEnvironment in Spec11Pipeline (#1365)
After #1348 it is no longer necessary to use AppEngineEnvironment in
Beam pipelines. In tests it is taken care of by the
DatastoreEntityExtension whereas on Dataflow the
RegistryPipelineWorkerInitializer does the same initialization for Ofy.
2021-10-02 19:23:09 -04:00
Rachel Guan
65ef18052b
Add autorenews to URS (#1343)
* Add autorenews to URS

* Add autorenews to existing xml files for test cases
2021-10-01 19:11:46 -04:00
Lai Jiang
f7938e80f7
Streamline how to fake an App Engine environment (#1348)
Both `DatastoreEntityExtension.PlaceholderEnvironment` and `AppEngineEnvironment` does the same thing, so there is no point having both of them exist. To use `AppEngineEnvionrment` as an autoclosable requires the user to be mindful of where a fake App Engine environment is required. It is better to set this either in the `DatastoreEntityExtension` for tests, or in the worker initializer in Beam. It also makes it easier to remove the fake environment when we are completely datastore free.

Also made a change to how `IdService` allocate Ids in Beam.
<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1348)
<!-- Reviewable:end -->
2021-10-01 16:46:46 -04:00
Lai Jiang
d8b3a30a20
Rename UpdateKmsKeyringCommand (#1353)
This brings it in line with GetKeyringSecretCommand. We still need to
remove the rest of remaining Cloud KMS related code in the future.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1353)
<!-- Reviewable:end -->
2021-10-01 16:45:45 -04:00
sarahcaseybot
93715c6f9e
Add VKey workaround to spec11 pipeline (#1339)
* Add VKey workaround to spec11 pipeline

* Parallelize entity loading
2021-10-01 15:21:16 -04:00
Rachel Guan
90cf4519c5
Add a cron job to periodically empty out fields on deleted entities t… (#1303)
* Add a cron job to periodically empty out fields on deleted entities that are at least 18 months old

* Process ContactHistory entities via batching

* Improve test cases by not making assertions in a loop
2021-09-30 15:17:37 -04:00
Michael Muller
3a177f36b1
Make :core:cleanTest depend on FilterTests (#1342)
* Make :core:cleanTest depend on FilterTests

The "cleanTest" target doesn't work for our specialized tests derived from
FilterTest.  Make them all explicit dependencies of cleanTest so we can reset
the tests from a single target.
2021-09-30 10:46:36 -04:00
Lai Jiang
fbbe014e96
Make it possible to stage a single Beam pipeline (#1351) 2021-09-29 18:27:23 -04:00
Ben McIlwain
b05b77cfd1
Add/use more DatabaseHelper convenience methods (#1327)
* Add/use more DatabaseHelper convenience methods

This also fixes up some existing uses of "put" in test code that should be
inserts or updates (depending on which is intended). Doing an insert/update
makes stronger guarantees about an entity either not existing or existing,
depending on what you're doing.

* Convert more Object -> ImmutableObject

* Merge branch 'master' into tx-manager-sigs

* Revert breaking PremiumListDao change

* Refactor more insertInDb()

* Fight more testing errors

* Merge branch 'master' into tx-manager-sigs

* Merge branch 'master' into tx-manager-sigs

* Merge branch 'master' into tx-manager-sigs

* Merge branch 'master' into tx-manager-sigs

* Add removeTmOverrideForTest() calls

* Merge branch 'master' into tx-manager-sigs
2021-09-28 17:16:54 -04:00
gbrodman
cc062e3528
Reduce # shards in CreateSyntheticHistoryEntriesAction (#1344)
We need these to get created (we are blocked from moving to SQL until 30
days after their creation) so reduce this to 3 in the hopes of avoiding
the SQL overloads while we debug why those are occurring in the first
place.
2021-09-28 10:46:50 -04:00
sarahcaseybot
de434f861f
Migrate ICANN activity reports to Cloud SQL on BQ (#1332)
* Migrate ICANN activity reports to Cloud SQL on BQ

* Fix data set name
2021-09-27 15:27:20 -04:00
Ben McIlwain
3caee5fba7
Improve some log messages for readability/consistency (#1333)
* Improve some log messages for readability/consistency

* Address code review comments
2021-09-27 11:35:14 -04:00
Ben McIlwain
ff3c848def
Add handling for UpdateAutoTimestamp when not in a transaction (#1341)
* Add handling for UpdateAutoTimestamp when not in a transaction

It's not clear why this is sometimes causing test flakes, but getting better
logging involved should help clear it up.

This also changes AppEngineExtension to insert without reloading the initial
test data, rather than putting it (potentially involving a merge) and reloading
it in a separate transaction. This should hopefully reduce the chance of weird
conflicts.
2021-09-27 11:32:15 -04:00
Rachel Guan
f0b3be5bb6
Improves test file for SendExpiringCertificateNotificationEmailAction (#1335)
* Improves test cases for SendExpiringCertificateNotificationEmailAction
2021-09-27 09:56:16 -04:00
gbrodman
18b808bd34
Fix injection with BackfillRegistryLocksCommand (#1337)
It would have been nice if this had failed at compile-time rather than
an NPE, but we need to make sure to specify that we need to inject this
command to get e.g. the random string generator

In addition, print out only the names of the failed domains (rather than
the entire domain object) for readability.
2021-09-24 14:08:30 -04:00
Lai Jiang
d7689539d7
Remove mention of bazel run (#1340)
Also provides a workaround in the error message.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1340)
<!-- Reviewable:end -->
2021-09-24 11:44:27 -04:00
Lai Jiang
c14ce6866b
Remove remnants of JUnit 4 rules (#1336)
<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1336)
<!-- Reviewable:end -->
2021-09-24 06:35:35 -04:00
Lai Jiang
fc7db91d70
Consolidate the use of URL parameters to specify database override (#1331)
There are actions for which we want to provide an override for the database
to use, like when launching Spec11 and Invoicing pipelines. It make sense to
consolidate around the same parameter provided from the same module for
consistency in all cases, instead of defining an override for each action.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/google/nomulus/1331)
<!-- Reviewable:end -->
2021-09-22 20:01:19 -04:00
Weimin Yu
3d8aa85d63
Fix ReadOnlyCheckingQuery's streaming method (#1329)
* Fix ReadOnlyCheckingQuery's streaming method

Following up to PR 1314: fix one more query defaulting to List when
stream() is invoked.
2021-09-21 15:40:50 -04:00
gbrodman
e14cd8bfa2
Add locking and a response in ReplicateToDatastoreAction (#1328)
* Add locking and a response in ReplicateToDatastoreAction

The response is necessary to get nicer logs in GAE and nicer cron job
behavior.

In addition:
- fix issues where locks would be backed up and replayed to Datastore
(they shouldn't be replayed)
- do ignore-read-only writes when replaying the transactions
2021-09-21 10:12:27 -04:00
Michael Muller
82e8641816
Fix javadoc problems with SoyInfo and subprojects (#1326)
* Fix javadoc problems with SoyInfo and subprojects

The *SoyInfo.java files generated by the soy compiler contain deprecation
warnings with links to files that are not imported.  This causes a javadoc
warning.  Temporarily fix this by replacing the link tags with "LINK".  This
also allows us to remove the exclusion of these files, which is a bit nicer.

Also disable javadoc tasks from subprojects.  These just break because they
don't have access to the legacy javadoc classes in the root.
2021-09-20 10:35:07 -04:00
gbrodman
7461ada0fb
Include Cursor in the initial SQL population (#1323)
The test for this also required a bit of a fix in the Cursor scope
initialization. If you persist a Key<?> in some object in Datastore, it
persists just the standard data you'd expect, basically the parent, the
kind, and the object's ID/name. Then, when you load it back in from
Datastore it uses the app ID of whatever environment you're loading in
(the Key contains this info even though it isn't included in the
toString() of Key)

If you persist the websafe string format of a Key (which is what we do
for Cursors), it includes the app ID so when you load it, it contains
the old app ID not the new one (if the app ID has changed).

In the pipelines, we use the standard default environment which has a
different app ID from the test environment that's set up by the
DatastoreEntityExtension.

As a result, we should check in Cursor to see if the key is *any*
cross-tld-key, rather than the exact one that exists within this app
environment.
2021-09-19 09:23:02 -04:00
Ben McIlwain
d91ca0eb8a
Clean up tx manager insert() signature and add convenience helper method (#1325)
* Clean up tx manager insert() signature and add convenience helper method

This is the first of a series of PRs to clean up the type signatures on the
TransactionManager methods (which are way too generic), along with creating some
helper methods for use in tests only that don't require creating transactions
all over the place, thus reducing visual noise at callsites. This first method
is DatabaseHelper.insertInDb(), but there will be plenty of others. Note that
this is only for the Cloud SQL transaction manager -- I'm not bothering to
migrate any Datastore-only code, as that will be going away soon enough.
2021-09-17 14:45:07 -04:00
gbrodman
12dac76dc8
Skip synthetic history entries for resources that don't need them (#1320)
* Skip synthetic history entries for resources that don't need them

The reason for creating synthetic history entries is so that we can
guarantee that each EppResource's most recent *History object contains
that resource at that point in time. If the most recent *History object
in SQL contains that resource already, there is no need to create a
synthetic *History object for that resource.
2021-09-17 12:10:15 -04:00
Michael Muller
ad6471b3fd
Clean up a few lint warnings (#1324)
The build is generating the following lint warnings:

core/src/main/java/google/registry/flows/certs/CertificateChecker.java:246:
warning: [ReferenceEquality] Compariso
n using reference equality instead of value equality
        && (lastExpiringNotificationSentDate == START_OF_TIME
                                             ^
    (see https://errorprone.info/bugpattern/ReferenceEquality)
core/src/test/java/google/registry/backup/ReplayCommitLogsToSqlActionTest.java:350:
warning: [UnnecessaryParenthes
es] These grouping parentheses are unnecessary; it is unlikely the code will
be misinterpreted without them
        .that(jpaTm().transact((() -> jpaTm().loadByEntity(contactResource))))
2021-09-17 09:15:48 -04:00