* Use Gradle dependency dynamic versioning
Use dynamic versioning for Gradle dependencies when possible.
Please refer to go/dr-dependency-upgrade for more information about the
automation plan.
This PR calls out all dependencies that must be pinned to specific
versions for various reasons. The remaining ones are converted to
open-ended version ranges ("[version_str,)").
* Add new columns to BillingEvent.java
* Improve PR and modifyJodaMoneyType to handle null currency in override
* Add test cases for edge cases of nullSafeGet in JodaMoneyType
* Improve assertions
* Bump flogger and beam dependency versions
Beam 2.34.0 -> 2.37.0
Flogger 0.7.3 -> 0.7.4
Intellij keeps getting confused about which version of Flogger we're
bringing in. Even though we had previously locked Flogger to 0.7.3, for
some reason it was still bringing in the Beam transitive dependency of
0.6.0 which was causing the a bunch of class initialization errors.
Bumping Beam to 2.34.0 bumps the transitive dependency to 0.7.4 so we
can always use that.
These are useful for the purposes of filtering by one-time/multi-use tokens, and
for determining which one-time tokens have been used (and if so, for which
domain).
* Add 3 more SQL indexes to the Host table
These indexes on creationTime, deletionTime, and currentSponsorRegistrarId are
present on the other two EPP resource tables (Domain and Contact), and are
useful for a wide variety of operations/analytics queries.
The query analyzer identified this is a missing index on the BillingEvent table,
and I added it for recurrences and cancellations as well as it's likely to be a
problem for them too. "Give me all the billing events associated with a given
domain by its repo ID" seems like a pretty common use case for the DB (and does
appear to be used by our invoicing pipeline).
This is a follow-up to PR #1545.
These indexes were identified as missing by PostgreSQL's query analyzer in our
sandbox environment (where we get enough realistic EPP traffic to identify these
deficiencies).
Note that a lot of the new indexes being named have to use the DB representation
of the column name because they are either embedded or subclassed entities,
whereas most of the existing ones are able to simply refer to Java field names.
This is the Java schema follow-up PR to PR #1541, which is what added the
actual DB changes through Flyway scripts.
* Reorganize new schema changes
Reorganized new schema changes and make each flyway script update a
single table.
Each flyway script is executed in a single database transaction so that
the script can be rolled back in one shot. It acquires a shared lock on
all tables touched by the script. This is deadlock-prone because in a
busy database, there may be user queries that attempt to lock the same
set of tables, but in different order. By limiting each script to one
table, we avoid the problem.
We should have some a presubmit check to enforce this rule.
All changes have been deployed to Sandbox out-of-band. When doing so,
we changed all CREATE INDEX statements to CREATE INDEX IF NOT EXISTS.
Future deployments should be able to proceed normally.
* Revise Host index on inet_addresses
The index on the 'inet_addresses array column should be of gin or gist
type, which index individual array elements. We use gin for now since
host updates are not often, and gin has better accuracy.
Since flyway script V108__... has not been deployed, we edit the file
in place instead of adding a new script.
This will be followed up with a modified query that can take advantage
of the gin index. Until then we don't expect to see performance
improvement.
The suspected bottlenect query in the whois path is:
select * from "Host" where 'non-ip-string' = any(inet_address) and
deletion_time < now();
It needs to be revised into:
select * from "Host" where array['non-ip-string'] <@ inet_address and
deletion_time < now();
The combined change reduces the query time from 90ms to 30ms in Sandbox,
and from 150ms to 40ms in production.
It is unclear if this solves all problem with whois latency.
* Add deletionTime/inetAddresses indexes to Host table to support WHOIS
Weimin identified these as missing, and being the cause of slowdowns in
NameserverLookupByIpCommand that we're seeing in sandbox.
This is the first of two PRs, adding just the Flyway/schema changes. The
second PR adding the Java object model changes is #1547.
* Add domainRepoId indexes to billing events #1544
The query analyzer identified this is a missing index on the BillingEvent table,
and I added it for recurrences and cancellations as well as it's likely to be a
problem for them too. "Give me all the billing events associated with a given
domain by its repo ID" seems like a pretty common use case for the DB (and does
appear to be used by our invoicing pipeline).
This is the first of two PRs that makes just the DB changes. The second PR
(#1544) will add the Java code changes, and will be committed after this one is
deployed.
* Add 9 more indexes to SQL schema
This indexes were identified as missing by PostgreSQL's query analyzer in our
sandbox environment (where we get enough realistic EPP traffic to identify these
deficiencies).
This is the first of two PRs -- the second PR (#1540) will be merged only after
this one is live in production. Note that this is the PR that actually modifies
the database though, so once this one is deployed we will already have the
benefit of the new indexes.
* Add an index on Host.host_name column
This field is queried during host creation and needs an index to speed
up the query.
Since Hibernate does not explicitly refer to indexes, we can change the
code and schema in one PR.
This version of Beam does not have an explicit dependency on log4j.
There are a couple of other things that need to change due to the
upgrade.
1) The new version pulls in a dependency that is not on Maven Central
but on packages.confluent.io, so we need to explicitly add this repo.
2) The new version has a dependency on flogger 0.6 anb above , which removed
the LoggerConfig class (see google/flogger#142).
We therefore backported the class. In the long term we should do what
was suggested in the issue and use the normal JDK Logger config
directly.
3) The intSqlPipeline dependency graph also needs to be updated.
<!-- 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/1472)
<!-- Reviewable:end -->
We can handle it the same way that we handle UpdateAutoTimestamp, where
we simply populate it in SQL if it doesn't exist. This has the following
benefits:
1. The converter is unnecessary code
2. We get non-null column definitions for free (overridden in
EppResource to allow null creation times so that legacy *History objects
can contain null in that field
3. More importantly, this allows us for proper SQL->DS replay. If the
field is filled out using a converter (as before this PR) then the field
is only actually filled out on transaction commit (rather than when the
write occurs within the transaction). This means that when we serialize
the Transaction object during the transaction (the data that gets
replayed to Datastore), we are crucially missing the creation time.
If the creation time is written on commit, we have to start a new
transaction to write the Transaction object, and it's an absolute
necessity that the record of the transaction be included in the
transaction itself so as to avoid situations where the transaction
succeeds but the record fails.
If the field is filled out in a @PrePersist method, crucially that
occurs on the object write itself (before transaction commit).
* 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.
* Add indexes to DomainHistory sub tables
Add indexes to DomainTransactionRecord and DomainDsDataHistory to speed
up query for DomainHistory. Without these indexes, DomainHistory loading
is extremely slow: 2 QPS with current production data.
* Add the DNS refresh request time field to the Domain tables
This isn't used yet, but it will eventually be the replacement for the dns-pull
task queue once we get further in the migration.
* Merge branch 'master' into domain-dns-dirty
* Add the domain DNS refresh request time field to the DB schema
This isn't used yet, but it will eventually be the replacement for the dns-pull
task queue once we get further in the migration.
* Remove index
* Store DatabaseMigrationSchedule in SQL instead of Datastore
This requires messing around with some of the JPA unit test rule
creation since it requires saving / retrieving the schedule pretty much
always (which itself includes the hstore extension).
* Remove KmsSecret model entities
Now that we have been using the SecretManager for almost a month now,
remove the KmsSecret and KmsSecretRevision entities from Java code base.
A follow-up PR will drop the relevant tables in the schema.
Also removed a few unused classes in the beam package.
The API provided by the GAE SDK will not be available outside GAE
runtime. This presents a problem when we migrate off of GAE. More
pressingly, the RDE pipeline migration to Beam requires that we write to
GCS on GCE. Previously we were able to sidestep the issue by delegating
the writes to FileIO provided by Beam, which knows how to write to GCS.
However the RDE pipeline cannot use FileIO directly as it needs to write
to multiple files in one go and explicit use of GCS API is needed.
An unfortunate side effect of the API migration is that the new testing
library contains a bug which makes serializing GcsUtils impossible. It
is fixed upstream but not released yet. The fix has been backported for
the time being.
<!-- 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/1228)
<!-- Reviewable:end -->
There is some complication regarding how the
CancellationMatchingBillingEvent of the generated OneTime can be
reconstructed when loading from SQL. I decided to only address it in
testing as there is no real value to fully reconstruct this VKey in
production where we are either in SQL or Ofy mode, both never in both.
Therefore the VKey in a particular mode only needs to contain the
corresponding key in order to function.
<!-- 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/1181)
<!-- Reviewable:end -->
* Safely lazy load claims and reserved lists
This moves the entries of all of these lists into "insignificant" fields and
manages them explicitly.
* Additional fixes
Fix a few problems that came up in the merge or weren't caught in earlier
local test runs.
* Changes for review
- removed debug code
- added comments
- improved some methods that were loading the entire claims list
unnecessarily.
* Fixed javadoc links
* Reformatted
* Minor fix for review
* Make PremiumList.labelsToPrices "insignificant"
Add the ImmutableObject.Insignificant annotation to labelsToPrices and also
mark it as Transient. In order to do lazy-loads on this field, we need to do
so explicitly: doing otherwise breaks the immutability contract and prevents
detaching the object upon load.
Note that this is an expedient solution to this problem, but not the optimal
one. Ideally, the disassociation between PremiumList and its PremiumEntry's
would be more explicit. However, breaking labelsToPrices out would at minimum
require reworking the Create/UpdatePremiumList commands, which currently rely
on passing around a self-contained PremiumList object, both from the parser
interfaces and to the database.
If this approach is acceptable, we can apply it to ReservedList and ClaimsList
as well (though it may be easier to break the association in those cases).
* Fix premium list "delete" to support a test
* Fix a few more tests
* Changes for review (updated javadocs)
* Minor fixes
* Updated getLablesToPrices() comment
* Format fixes, fixed PremiumEntry interfaces
PremiumEntry can now be SQL only.
* Add loadOnlyOf method to tm()
In addition there's a bit of a refator of SqlReplayCheckpoint to make it
more in line with the other singletons. This method is useful for the
singleton classes where we expect at most one entity to exist, e.g.
ServerSecret.
* Combine the two Lock classes into one class
This allows us to remove the DAO and to just treat locks the same as we
would treat any other object -- generically grabbing them from the
transaction manager.
We do not need to be concerned about the changeover between Datastore
and SQL because we assume that any such changeover will require
sufficient downtime that any currently-valid acquired locks will expire
during the downtime. Otherwise, we could get into a situation where an
action has acquired a particular lock in Datastore but not SQL.
Nothing super crazy here other than persisting the entity changes in
DomainDeleteFlow at the end of the flow rather than almost at the end.
This means that when we return the results we give the results as they
were originally present, rather than the subsequently-changed values.
* Defer all foreign keys in SQL
The main difference here is that the constraint violation exceptions
won't be thrown until the transaction is completed, rather than when the
insert is first performed within the transaction. We get the same error
message either way. The primary benefit to this is that when dealing
with large operations inside a single transaction (flows), we don't need
to worry about the order of insertions of removals with regards to
foreign keys.
* Upgrade testcontainers to work around a race
testcontainers 1.15.? has a race condition that occassionally causes deadlocks.
This can be worked around by upgrading to 1.15.2 and set transport type to
http5.
See https://github.com/testcontainers/testcontainers-java/issues/3531
for more information.
There are two changes that are not lockfiles:
- dependencies.gradle
- java_common.gradle
* Convert TmchCrl and ServerSecret to cleaner tm() impls
When I implemented this originally I knew a lot less than I know now
about how we'll be storing and retrieving these singletons from SQL. The
optimal way here is to use the single SINGLETON_ID as the primary key,
that way we always know how to create the key that we can use in the
tm() retrieval.
This allows us to use generic tm() methods and to remove the handcrafted
SQL queries.
* Add a beforeSqlSave callback to ReplaySpecializer
When in the Datastore-primary and SQL-secondary stage, we will want to
save the EppResource-at-this-point-in-time field in the *History
objects so that later on we can examine the *History objects to see what
the resource looked like at that point in time.
Without this PR, the full object at that point in time would be lost
during the asynchronous replay since Datastore doesn't know about it.
In addition, we modify the HistoryEntry weight / priority so that
additions to it come after the additions to the resource off of which it
is based. As a result, we need to DEFER some foreign keys so that we can
write the billing / poll message objects before the history object that
they're referencing.
