New store for unavailable parties - #6725
Conversation
Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com>
…ulien/6702-unavailable-parties-stores
martinflorian-da
left a comment
martinflorian-da
left a comment
There was a problem hiding this comment.
Thanks!
It mostly looks good to me; I'm mostly unsure about the indexes / query plans; maybe let's do some experiments even if it's a bit annoying?
|
|
||
| -- Index for the expiry check per party | ||
| create index dso_unavailable_parties_pid_exp | ||
| on dso_unavailable_parties (party_id, (updated_at + ignore_duration)); |
There was a problem hiding this comment.
I think the party_id might be actually counterproductive here.
Honestly I think it might be a good investment to fill up a database with synthetic data and get some query plans to confirm the indexes work as intended...
There was a problem hiding this comment.
Fair point but #5775 tests slightly different queries and specifically has no query that looks like listParties, AFAICT...
If the investment to do synthetic tests seems disproportionate, let's at least make sure to check query plans once this hits CILR and has some actual data.
There was a problem hiding this comment.
check query plans once this hits CILR and has some actual data.
will definitely do that!
…ulien/6702-unavailable-parties-stores
Thanks for the review. The query plans were tested in #5775 |
Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com>
Signed-off-by: Julien Tinguely <julien.tinguely@digitalasset.com> Signed-off-by: jarekr-da <jaroslaw.ratajski@digitalasset.com>
2 participants
fixes #6702
fixes #6701
Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./upgrade_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./lsu_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines