[LWMeta] TimeLock API changes #6660

Toemmsche · 2023-07-10T15:00:24Z

General

Before this PR:
ConjureLockRequest uses set<ConjureLockDescriptor>
After this PR:

Conjure excludeEmptyOptionals = true
New field optional<ConjureLockRequestMetadata> in ConjureLockRequest
ConjureLockRequest uses list<ConjureLockDescriptor> instead
If metadata is empty after lock watch filtering, it is omitted

==COMMIT_MSG==
[LWMeta] Add metadata to ConjureLockRequest
==COMMIT_MSG==

Priority:
P1
Concerns / possible downsides (what feedback would you like?):
Looking at some serialized ConjureLockRequest objects, there is an argument to be made to simplify the ChangeMetadata format to always include the old and new value. The conjure encoding for union types is out of our control and - frankly - quite poor.

Is documentation needed?:
If we have documentation for the TimeLock API, yes!

Compatibility

Does this PR create any API breaks (e.g. at the Java or HTTP layers) - if so, do we have compatibility?:
Due to us not using --strictObjects and using excludeEmptyOptionals = true as of this PR, we have compatibility.
Does this PR change the persisted format of any data - if so, do we have forward and backward compatibility?:
No
The code in this PR may be part of a blue-green deploy. Can upgrades from previous versions safely coexist? (Consider restarts of blue or green nodes.):
Yes, as long as nodes on newer versions do no forward requests to nodes of older versions.
Does this PR rely on statements being true about other products at a deployment - if so, do we have correct product dependencies on these products (or other ways of verifying that these statements are true)?:
TBD
Does this PR need a schema migration?
no

Testing and Correctness

What, if any, assumptions are made about the current state of the world? If they change over time, how will we find out?:

What was existing testing like? What have you done to improve it?:
I have added tests to the existing LockWatchEventIntegrationTest to verify that metadata passed to TimeLock is visible to transactions.
If this PR contains complex concurrent or asynchronous code, is it correct? The onus is on the PR writer to demonstrate this.:
N/A
If this PR involves acquiring locks or other shared resources, how do we ensure that these are always released?:
N/A

Execution

Metrics for TimeLock are added in #6665
If this PR does not work as expected, how do I fix that state? Would rollback be straightforward?:
Yes
If the above plan is more complex than “recall and rollback”, please tag the support PoC here (if it is the end of the week, tag both the current and next PoC):

Scale

Would this PR be expected to pose a risk at scale? Think of the shopping product at our largest stack.:
This PR does introduce some additional conversions from List<LockDescriptor> to Set<LockDescriptor> and back. These are necessary to facilitate the index encoding format for metadata.
Would this PR be expected to perform a large number of database calls, and/or expensive database calls (e.g., row range scans, concurrent CAS)?:
No
Would this PR ever, with time and scale, become the wrong thing to do - and if so, how would we know that we need to do something differently?:
If the aforementioned conversions are performance bottlenecks, we should consider a separate endpoint for lock requests with metadata.

Development Process

Where should we start reviewing?:

If this PR is in excess of 500 lines excluding versions lock-files, why does it not make sense to split it?:
We exceed the 500 line threshold because of adding an old version of a conjure-generated class

Toemmsche · 2023-07-12T10:26:04Z

...pl-shared/src/main/java/com/palantir/atlasdb/debug/LockDiagnosticConjureTimelockService.java

is this class still in use?

Toemmsche · 2023-07-12T10:29:41Z

lock-api/src/test/java/com/palantir/lock/client/ConjureLockRequestTest.java

This test is merely for compatibility + serializability checking so we don't have to test too many edge cases here. Anything related to actual index encoding (e.g. correctness checks) is handled by the conjure utilities.

Toemmsche · 2023-07-12T10:31:08Z

timelock-api/src/main/conjure/timelock-api.yml

@@ -139,13 +139,14 @@ types:
          requestId:
            type: uuid
            safety: safe
-          lockDescriptors: set<ConjureLockDescriptor>
+          lockDescriptors: list<ConjureLockDescriptor>


as discussed during the RFC review: This change is compatible on the wire

Toemmsche · 2023-07-12T10:35:16Z

timelock-api/src/main/conjure/timelock-api.yml

          acquireTimeoutMs:
            type: integer
            safety: safe
          clientDescription:
            type: optional<string>
            safety: unsafe
+          metadata: optional<ConjureLockRequestMetadata>


Interestingly enough, even though https://github.com/palantir/conjure/blob/master/docs/spec/wire.md#42-servers-reject-unknown-fields dictates that servers must reject unknown fields, all conjure object builder are actually annotated with @JsonIgnoreProperties(ignoreUnknown = true) meaning that they ignore all unknown properties. If we trust the spec, this change would require a product dependency, but the generated conjure code says otherwise.

Toemmsche · 2023-07-12T10:36:09Z

timelock-impl/src/main/java/com/palantir/atlasdb/timelock/ConjureTimelockResource.java

            IdentifiedLockRequest lockRequest = ImmutableIdentifiedLockRequest.builder()
-                    .lockDescriptors(fromConjureLockDescriptors(request.getLockDescriptors()))
+                    .lockDescriptors(lockDescriptors)


Even though we are passing a list to field that expects a set, immutable types always build an immutable version of the container type anyways (to guarantee immutability)

Toemmsche · 2023-07-12T10:36:57Z

...erver/src/integTest/java/com/palantir/atlasdb/timelock/AsyncTimeLockServiceMetadataTest.java

Refactor because we moved some code to the LockWatchIntegrationTestUtilities class

Toemmsche · 2023-07-12T12:42:44Z

...erver/src/integTest/java/com/palantir/atlasdb/timelock/AsyncTimeLockServiceMetadataTest.java

-            return Optional.empty();
-        }
-    }
+    private enum AssertSuccessVisitor implements LockWatchStateUpdate.Visitor<LockWatchStateUpdate.Success> {


This is pretty much duplicated from UpdateVisitors, but that class is in another test module. We could move it to a non-test module (ugly because test code now lives among production code) or move this test class over (but then we couldn't access LockWatchIntegrationTestUtilities). I am not too offended by this just being here.

Change lock descriptor container type to list Add test for backwards compatibility of conjure lock request deserialization in TimeLock Extend LockWatchEventIntegrationTest with tests for metadata (it should be visible for transactions)

[skip ci]

Refactor test file naming

changelog-app · 2023-07-18T14:45:12Z

Generate changelog in `changelog/@unreleased`

What do the change types mean?

feature: A new feature of the service.
improvement: An incremental improvement in the functionality or operation of the service.
fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
and external consumers of the service's API (e.g. customer-written software or integrations).
deprecation: Advertises the intention to remove service functionality without any change to the
operation of the service itself.
manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
performing database surgery, ...) at the time of upgrade for it to succeed.
migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

❗The break and manual task changelog types will result in a major release!
🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
✨ All others will result in a minor version release.

Type

Description

[LWMeta] Add metadata to ConjureLockRequest

Check the box to generate changelog(s)

Generate changelog entry

[skip ci]

Sam-Kramer

Looks good! mostly minor stuff

Sam-Kramer · 2023-07-18T15:03:32Z

...pl-shared/src/main/java/com/palantir/atlasdb/debug/LockDiagnosticConjureTimelockService.java

@@ -97,24 +99,26 @@ public LeaderTime leaderTime(AuthHeader authHeader, String namespace) {

    @Override
    public ConjureLockResponse lock(AuthHeader authHeader, String namespace, ConjureLockRequest request) {
+        Set<ConjureLockDescriptor> lockDescriptors = Set.copyOf(request.getLockDescriptors());


Why are we now "copying" the request lock descriptors?

Downstream methods like ClientLockDiagnosticCollector#collect require a set, but ConjureLockRequest.lockDescriptors is now a list. Since this class is deprecated, I don't think it is worth the effort to refactor all the downstream methods and immutable objects to accept lists. Also, having set semantics makes more sense in this context and we merely have to do the conversion because of the wire format.

That said, we should use ImmutableSet.copyOf here

Sam-Kramer · 2023-07-18T15:16:32Z

lock-api/src/main/java/com/palantir/lock/client/ConjureLockRequests.java

@@ -62,6 +73,14 @@ private static Set<ConjureLockDescriptor> toConjure(Set<LockDescriptor> lockDesc
                .collect(Collectors.toSet());
    }

+    private static List<ConjureLockDescriptor> toConjure(List<LockDescriptor> lockDescriptors) {


(no action: https://docs.oracle.com/javase/8/docs/api/java/util/stream/package-summary.html#Ordering)

Sam-Kramer · 2023-07-18T15:23:42Z

lock-api/src/test/java/com/palantir/lock/client/ConjureLockRequestTest.java

+            ObjectMappers.newServerObjectMapper().enable(SerializationFeature.INDENT_OUTPUT);
+    // This mapper is used to ensure that two JSONs are equal excluding indentation
+    private static final ObjectMapper VERIFYING_MAPPER = new ObjectMapper();
+


Given our conversation around "dependency ordering", should we write a test proving that the old ConjureLockRequest will be able to handle metadata, whether or not it's specified? A bit ugly, as we'll need to copy the old conjure-generated file, but probably worth it to as our proof it won't break things

Yeah I thought about this and found it acceptable in the LockEvent case since copying the class was trivial, but copying the generated class is a different beast. Given that we will not have product dependencies, though, I think it is acceptable.

I don't think it's the worst thing in the world for us to have a copy in the test folder here of the old-generated class. Granted, this is something we probably want anyways, as we have an invariant here which we must maintain, albeit super hard to break (our new format must be serializable by the old format).

Even though this becomes out-dated after this PR, we still allow the upgrade path from a really old version of timelock to a really new version of timelock in the future, so it's useful we have this test to ensure we don't break anything until we checkpoint timelock.

Sounds reasonable, I noticed that the test class is not really in the right module (should be co-located with timelock-api.yml), will move it

Sam-Kramer · 2023-07-18T15:29:15Z

lock-api/src/test/java/com/palantir/lock/client/ConjureLockRequestTest.java

+    }
+
+    private static Path getJsonPath(String jsonFileName) {
+        return Paths.get(BASE + jsonFileName + ".json");


this is fine, but I think you can also do Paths.get(BASE, jsonFileName + ".json")

will backport this suggestion for related files

Sam-Kramer · 2023-07-18T15:44:20Z

timelock-impl/src/main/java/com/palantir/atlasdb/timelock/ConjureTimelockResource.java

-    private static Set<LockDescriptor> fromConjureLockDescriptors(Set<ConjureLockDescriptor> lockDescriptors) {
-        Set<LockDescriptor> descriptors = Sets.newHashSetWithExpectedSize(lockDescriptors.size());
+    private static List<LockDescriptor> fromConjureLockDescriptors(List<ConjureLockDescriptor> lockDescriptors) {
+        List<LockDescriptor> descriptors = new ArrayList<>(lockDescriptors.size());


Do we need to make this List mutable? Otherwise I saw let's opt for ImmutableList.

Sam-Kramer · 2023-07-18T15:50:13Z

timelock-api/build.gradle

+
+conjure {
+    java {
+        excludeEmptyOptionals = true


Sam-Kramer · 2023-07-18T16:06:03Z

...k-server/src/integTest/java/com/palantir/atlasdb/timelock/LockWatchEventIntegrationTest.java

@@ -359,6 +387,30 @@ private Set<LockDescriptor> getDescriptors(TableReference tableRef, Cell... cell
                .collect(Collectors.toSet());
    }

+    private List<Optional<LockRequestMetadata>> lockAndGetAllLockEventMetadataForNewTransaction(
+            LockRequest lockRequest) {
+        txnManager.runTaskReadOnly(txn -> null);


Do we actually need this?

We don't need to actually run a transaction, but we need the currentVersion (line below)

Sam-Kramer · 2023-07-18T16:08:32Z

...k-server/src/integTest/java/com/palantir/atlasdb/timelock/LockWatchEventIntegrationTest.java

+        LockResponse response = txnManager.getTimelockService().lock(lockRequest);
+        // We need to clean up the lock, otherwise it will be held forever (the TimeLock cluster is static)!
+        txnManager.getTimelockService().unlock(ImmutableSet.of(response.getToken()));
+        OpenTransaction dummyTxn = startSingleTransaction();


nit: maybe a better name than dummyTxn

Sam-Kramer · 2023-07-18T16:11:11Z

...k-server/src/integTest/java/com/palantir/atlasdb/timelock/LockWatchEventIntegrationTest.java

@@ -359,6 +387,30 @@ private Set<LockDescriptor> getDescriptors(TableReference tableRef, Cell... cell
                .collect(Collectors.toSet());
    }

+    private List<Optional<LockRequestMetadata>> lockAndGetAllLockEventMetadataForNewTransaction(


Maybe worth just returning the Optional<LockRequestMetadata>, seeing as we only expect a single event anyways?

lockAndGetEventMetadataFromLockWatchLog ?

Sam-Kramer · 2023-07-18T16:12:41Z

...k-server/src/integTest/java/com/palantir/atlasdb/timelock/LockWatchEventIntegrationTest.java

@@ -26,6 +26,7 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.util.concurrent.Uninterruptibles;
+import com.palantir.atlasdb.encoding.PtBytes;


Do we want to be a bit more aggressive here with our testing?

I think any test we add will already be mostly by some other test. But given that this is the ultimate integration test, I would be fine with replicating/bundling some of them here.

I think a large-scale random test might make sense.

Thinking about this more, I would rather use this class for advanced testing using actual transactions once we have the necessary changes in Atlas. The two tests are added don't really fit the purpose of this class, but are good sanity checks. Thoughts on this approach?

I think that makes sense! Plus, this api is not really used anyways until that happens, so it's fine for this to merge beforehand.

… product dependency

Optimize metadata filtering in LockWatchingServiceImpl

Toemmsche · 2023-07-19T10:07:53Z

...ock-impl/src/main/java/com/palantir/atlasdb/timelock/lock/watch/LockWatchingServiceImpl.java

-                    return LockRequestMetadata.of(filteredLockMetadata);
+                    return filteredLockMetadata.isEmpty()
+                            ? Optional.empty()
+                            : Optional.of(LockRequestMetadata.of(filteredLockMetadata));


This is not necessary for correctness, but we should have this behavior for performance reasons

Rather than ternary I personally bias towards

if(filteredLockMetadata.isEmpty()) { return Optional.empty();` } return Optional.of(...)

BUT -- that said, I think ternary is also fine!

I can get behind it

Toemmsche · 2023-07-19T14:06:33Z

timelock-api/src/test/java/com/palantir/atlasdb/timelock/api/OldConjureLockRequest.java

This is copied from the latest develop , but I renamed it to avoid confusion

Sam-Kramer · 2023-07-19T14:31:48Z

timelock-api/src/test/java/com/palantir/atlasdb/timelock/api/OldConjureLockRequest.java

+@Unsafe
+@JsonDeserialize(builder = OldConjureLockRequest.Builder.class)
+@Generated("com.palantir.conjure.java.types.BeanGenerator")
+public final class OldConjureLockRequest {


Old is fine, but Legacy is a bit cleaner

Sam-Kramer · 2023-07-19T14:36:51Z

timelock-api/src/test/java/com/palantir/atlasdb/timelock/api/ConjureLockRequestTest.java

+        assertDeserializedEquals(OLD_CONJURE_LOCK_REQUEST_FILE, OLD_CONJURE_LOCK_REQUEST, OldConjureLockRequest.class);
+    }
+
+    // Requires excludeEmptyOptionals = true


Maybe reframe the comment to be more specific on what this means, i.e. we exclude optionals during serialization rather than marking them as null.

Sort of frustrating, as this is the ideal of as, so you can say "we expect the old and new conjure lock request to be identical as we do not serialize empty optionals".

It might be worth removing the as and to return the assertThat, to allow for these types of cases.

I think an improved comment is fine. Trying to get the assert here is especially ugly for assertSerializedEquals because we re-read the serialized form using a different mapper (so we can ignore any indentation differences).

Sam-Kramer · 2023-07-19T14:38:49Z

timelock-api/src/test/java/com/palantir/atlasdb/timelock/api/ConjureLockRequestTest.java

+        }
+    }
+
+    private static <T> void assertDeserializedEquals(String jsonFileName, T object, Class<T> clazz) {


also only other style nit, is that we have arguments reversed from assertSerializedEquals. I'd expect assertDeserializedEquals(T object, String jsonFileName, Class<T> clazz)

This was deliberate to maintain the convention that we do assertThat(actual).isEqualTo(expected). The deserialized JSON object is the actual value and the object is the expected value.

In assertSerializedEquals, the JSON file is the expected value.

ah, then that makes sense!

Sam-Kramer · 2023-07-19T14:44:02Z

timelock-impl/src/main/java/com/palantir/atlasdb/timelock/ConjureTimelockResource.java

-    private static Set<LockDescriptor> fromConjureLockDescriptors(Set<ConjureLockDescriptor> lockDescriptors) {
-        Set<LockDescriptor> descriptors = Sets.newHashSetWithExpectedSize(lockDescriptors.size());
+    private static List<LockDescriptor> fromConjureLockDescriptors(List<ConjureLockDescriptor> lockDescriptors) {
+        ImmutableList.Builder<LockDescriptor> descriptors = ImmutableList.builder();


As we know the size here, let's use builderWithExpectedSize.

Sam-Kramer

lgtm

Toemmsche added the do not merge label Jul 10, 2023

Toemmsche changed the base branch from develop to tpapke/metadata/wire-format-lockevent-serdes July 10, 2023 15:00

Toemmsche force-pushed the tpapke/metadata/timelock-api-changes branch 2 times, most recently from 224c741 to 7182be2 Compare July 11, 2023 15:04

Toemmsche force-pushed the tpapke/metadata/wire-format-lockevent-serdes branch from b1ca4c2 to 80d9689 Compare July 11, 2023 15:23

Toemmsche force-pushed the tpapke/metadata/timelock-api-changes branch from 7182be2 to 42bce2e Compare July 12, 2023 09:54

Toemmsche force-pushed the tpapke/metadata/wire-format-lockevent-serdes branch from 80d9689 to 37e17e5 Compare July 12, 2023 09:55

Toemmsche commented Jul 12, 2023

View reviewed changes

Toemmsche requested a review from mdaudali July 12, 2023 10:38

Toemmsche force-pushed the tpapke/metadata/timelock-api-changes branch from 41d266c to a361c32 Compare July 12, 2023 12:29

Toemmsche commented Jul 12, 2023

View reviewed changes

Toemmsche force-pushed the tpapke/metadata/wire-format-lockevent-serdes branch from d02537d to 6902515 Compare July 12, 2023 17:50

Toemmsche force-pushed the tpapke/metadata/timelock-api-changes branch from 24ca170 to 2510b74 Compare July 12, 2023 17:51

Toemmsche force-pushed the tpapke/metadata/wire-format-lockevent-serdes branch from 6902515 to a4685ab Compare July 13, 2023 08:53

Toemmsche force-pushed the tpapke/metadata/timelock-api-changes branch from 2510b74 to 2ec8eef Compare July 13, 2023 09:40

Toemmsche force-pushed the tpapke/metadata/wire-format-lockevent-serdes branch 2 times, most recently from 4e93e58 to 4dd4296 Compare July 13, 2023 15:11

Toemmsche force-pushed the tpapke/metadata/timelock-api-changes branch 2 times, most recently from d5e8260 to 19dfcda Compare July 13, 2023 15:53

Toemmsche marked this pull request as ready for review July 14, 2023 10:37

Toemmsche requested a review from Sam-Kramer July 14, 2023 15:45

Toemmsche force-pushed the tpapke/metadata/timelock-api-changes branch from 12d352c to b63000e Compare July 18, 2023 14:06

Toemmsche added 3 commits July 18, 2023 15:28

Unlock after we are done

3d6e1fe

Convert to INSTANCE

867a5a1

Toemmsche and others added 2 commits July 18, 2023 15:29

Autorelease 0.895.0-rc1

b68c764

[skip ci]

Handle IOException

e5cbcf9

Refactor test file naming

Toemmsche force-pushed the tpapke/metadata/timelock-api-changes branch from b63000e to e5cbcf9 Compare July 18, 2023 14:29

Sam-Kramer changed the base branch from tpapke/metadata/wire-format-lockevent-serdes to develop July 18, 2023 14:45

Toemmsche and others added 2 commits July 18, 2023 15:58

use excludeEmptyOptionals = true

7e0e841

Autorelease 0.898.0-rc1

0b14a8a

[skip ci]

Sam-Kramer reviewed Jul 18, 2023

View reviewed changes

Toemmsche added 4 commits July 18, 2023 17:33

WIP

14484da

Add old version of conjure lock request and test that we don't need a…

7b67b7d

… product dependency

Make list immutable

6cdbdcc

Address comments

29c8bf6

Optimize metadata filtering in LockWatchingServiceImpl

Toemmsche requested a review from Sam-Kramer July 19, 2023 09:18

svc-changelog and others added 3 commits July 19, 2023 09:57

Add generated changelog entries

f7b4043

Add generated changelog entries

eb224fd

Move ConjureLockRequestTest to conjure module

57a1fe4

Toemmsche commented Jul 19, 2023

View reviewed changes

Toemmsche removed the do not merge label Jul 19, 2023

Toemmsche commented Jul 19, 2023

View reviewed changes

Sam-Kramer reviewed Jul 19, 2023

View reviewed changes

Address comments

645d977

Toemmsche requested a review from Sam-Kramer July 19, 2023 15:31

Sam-Kramer approved these changes Jul 19, 2023

View reviewed changes

Toemmsche added 2 commits July 19, 2023 17:52

Rename "old" to "legacy"

7702273

fix

a8380ed

Toemmsche added the merge when ready label Jul 19, 2023

bulldozer-bot bot merged commit bfe7051 into develop Jul 19, 2023

[LWMeta] TimeLock API changes #6660

[LWMeta] TimeLock API changes #6660

Conversation

Toemmsche commented Jul 10, 2023 • edited Loading

General

Compatibility

Testing and Correctness

Execution

Scale

Development Process

Choose a reason for hiding this comment

Toemmsche Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changelog-app bot commented Jul 18, 2023 • edited by Toemmsche Loading

Generate changelog in changelog/@unreleased

Sam-Kramer left a comment

Choose a reason for hiding this comment

Sam-Kramer Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Toemmsche Jul 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Toemmsche Jul 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sam-Kramer left a comment

Choose a reason for hiding this comment

Toemmsche commented Jul 10, 2023 •

edited

Loading

Toemmsche Jul 12, 2023 •

edited

Loading

changelog-app bot commented Jul 18, 2023 •

edited by Toemmsche

Loading

Generate changelog in `changelog/@unreleased`

Sam-Kramer Jul 18, 2023 •

edited

Loading

Toemmsche Jul 19, 2023 •

edited

Loading

Toemmsche Jul 19, 2023 •

edited

Loading