Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CORE-6717 Add schemas for hosted identity reconciliation #1620

Merged
merged 14 commits into from
May 9, 2024

Conversation

YashNabar
Copy link
Contributor

@YashNabar YashNabar commented Apr 24, 2024

Introduces database and Avro schemas to enable persistence and reconciliation of locally-hosted identities.

  • Adds cluster-level tables hosted_identity and hosted_identity_session_key_info
  • Extends HostedIdentityEntry.avsc to include version (compatibility test added)
  • Adds config for hosted identity reconciliation

@corda-jenkins-ci02
Copy link
Contributor

corda-jenkins-ci02 bot commented Apr 24, 2024

Jenkins build for PR 1620 build 9

Build Successful:
Jar artifact version produced by this PR: 5.2.1.53-alpha-1715186958139

@corda-jenkins-ci02
Copy link
Contributor

corda-jenkins-ci02 bot commented Apr 24, 2024

Scanning for breaking API changes introduced by this PR

Scan Failed: https://ci02.dev.r3.com/job/Corda5/job/corda-api-compatibility/job/PR-1620/8/

If the breaking changes are intentional, run ./gradlew cementApi and get approval from the Corda team leads.

nikinagy
nikinagy previously approved these changes Apr 29, 2024
Copy link
Contributor

@nikinagy nikinagy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

owenstanford
owenstanford previously approved these changes Apr 30, 2024
Copy link
Contributor

@driessamyn driessamyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot go in 5.2 as patch releases must be able to be applied in place (without an upgrade process for DBs etc).

@nikinagy
Copy link
Contributor

This cannot go in 5.2 as patch releases must be able to be applied in place (without an upgrade process for DBs etc).

@driessamyn The main problem is that we were not persisting data in the DB, so in case of kafka data loss we lost everything.
The hosted identities topic was never backed up by any sort of DB entity. Hence we had no DB reconciliation in place.
I don't see how we could fix the SBI issue for clients without making any changes to the DB. Especially, since the main issue is that we are missing persistence for this data.

@@ -61,6 +61,13 @@
"minimum": 5000,
"maximum": 2147483647,
"default": 120000
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a heads up, after upgrade it's possible the config read by a worker will not yet have this property with a default propagated to it. The code must be able to handle this property is missing in 5.2.1. See https://r3-cev.atlassian.net/wiki/spaces/CB/pages/4564091182/Making+a+Config+Change for more details.

@driessamyn driessamyn dismissed their stale review May 2, 2024 08:31

Removing block as confirmed with Marcello (pending write-up of decision) that this can go in 5.2.1.

@YashNabar YashNabar dismissed stale reviews from owenstanford and nikinagy via 15b92b5 May 8, 2024 16:48
Copy link

sonarqubecloud bot commented May 8, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@corda-jenkins-ci02
Copy link
Contributor

Non-blocking downstream job failed for corda-non-functional-test

https://ci02.dev.r3.com/job/Corda5/job/corda-api-compatibility/job/PR-1620/8/ has failed for PR 1620 build 8

Please investigate if your changes may have broken compilation on https://github.com/corda/corda-non-functional-test

@corda-jenkins-ci02
Copy link
Contributor

Non-blocking downstream job failed for corda-e2e-test

https://ci02.dev.r3.com/job/Corda5/job/corda-api-compatibility/job/PR-1620/8/ has failed for PR 1620 build 8

Please investigate if your changes may have broken compilation on https://github.com/corda/corda-e2e-tests

Copy link
Contributor

@conalsmith-r3 conalsmith-r3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants