Implement String interning store by OriolMunoz-da · Pull Request #7111 · canton-network/splice

OriolMunoz-da · 2026-09-03T17:06:03Z

OriolMunoz-da · 2026-09-04T10:27:01Z

+  ) extends InternedStringStore
+      with NamedLogging {
+
+    override def getOrIntern(value: String)(implicit tc: TraceContext): Future[InternedId] = {


the query proposed in the design was:

with new_row as ( insert into interned_strings (value) values ($v) on conflict (value) do nothing returning id ) select id from new_row union all select id from interned_strings where value = $v limit 1;

this does not work with contention:

[info] - handle concurrent interning of the same value without cache *** FAILED *** [info] java.util.NoSuchElementException: empty result setSome( when invoking statement [info] with new_row as ( [info] insert into interned_strings (value) values (?) [info] on conflict (value) do nothing returning id [info] ) [info] select id from new_row union all select id from interned_strings where value = ? limit 1; [info] )

the reason for that is that if there's two queries, you can end up in this situation (as far as I understand):

q1 runs the insert successfully

q2 runs the insert which conflicts and returns nothing

q2 runs the select which returns nothing

q2 returns nothing

q1 commits

And q1 returns the right set?

yeah but q2 returns nothing

yes, q1 behaves properly, but q2 blows up because nothing is returned

yeah your version is safer, is fine.

but in the new way you can still have concurrent insert/select no? the flatmap is not a tx boundary or is it?

(if it is, then it explains it, otherwise you might still have a problem?)

ah ok by default that is autocommit per statement

the flatmap is not a tx boundary

it is; it's separate txs (storage.query twice, as opposed to DBIO composition)

ok cool thanks I get it

OriolMunoz-da · 2026-09-04T10:28:06Z

    metrics.UpdateHistory.assignments.mark()
-    sqlu"""
+    for {
+      _ <- DBIO.from(internTemplateId(templateId))


this is wrapping the future: i.e., it's not transactional with the rest of the statements. That should be fine since 99.99% of the time we're using the cache anyway

OriolMunoz-da · 2026-09-04T10:32:06Z

+
+  // This does not need to be transactional with the rest of the transactions in UpdateHistory
+  private def internTemplateId(identifier: Identifier)(implicit tc: TraceContext): Future[Unit] = {
+    // TODO (#6257): use the returned ids in the partitioned table


we can start to fill up the store now and use it later in the partitioned tables
from the scan pruning doc:

The dualWrite/readFromId rollout in the String Interning design covers only the legacy tables, which are dropped wholesale, so it is no longer needed.

ray-roestenburg-da · 2026-09-04T10:47:06Z

+          "fetchAllInternedStrings",
+        )
+        .map { all =>
+          if (all.size >= maxSize) {


why don't you just do a count(1) on interned_strings (or similar on metadata of postgres), and then compare so maxSize? Also, we do not necessarily need all strings in cache, this depends on access pattern, will this not just lead to unnecessary warnings?

from the design doc, this warning should never be hit and everything should be fine to be in the cache:

if it is hit, we have a problem

ah right, thanks lol

~~actually from this i realize i'm missing the package name and choice name in the updatehistory intern call~~ done

ray-roestenburg-da · 2026-09-04T11:17:46Z

      participantId = mkParticipantId(this.getClass.getSimpleName),
      updateStreamParty = dsoParty,
      backfillingRequired = UpdateHistory.BackfillingRequirement.BackfillingNotRequired,
+      internedStringStore = InternedStringStore.createWithoutWarmup(


Are you doing withoutWarmup in tests just to speed up the tests? Any idea how long this takes in ScanApp with the warmup?

Are you doing withoutWarmup in tests just to speed up the tests?

doesn't really matter, the DB will be empty there

Any idea how long this takes in ScanApp with the warmup?

it's a query fetching 0-1000 rows, that shouldn't be too long

so we don't need the withoutWarmup version?

I guess not, I can remove it

not a huge deal, was just wondering

it does have the annoyance that warmup returns a Future, whereas the other one can just return the cache. Will leave as-is unless you disagree

is fine, leave as is

ray-roestenburg-da

Looks good, thanks!

rautenrieth-da

Thanks!

rautenrieth-da · 2026-09-04T22:31:21Z

+        .query(
+          sql"""
+            insert into interned_strings (value) values ($value)
+            on conflict (value) do nothing returning id;


This might consume an entry from the auto-increment sequence on each read. We should still be fine for the next billion years before the bigint overflows.

rautenrieth-da · 2026-09-04T22:41:52Z

        config.acsStoreDescriptorUserVersion,
        config.txLogStoreDescriptorUserVersion,
      )
+      internedStringStore <- InternedStringStore.createAndWarmupCache(


Loading all interned strings from the database will delay application startup slightly, do you know by how much?

same answer as in #7111 (comment), there will be <10k strings that should fit in the cache and be trivially fetchable from a single query

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

OriolMunoz-da force-pushed the oriol/interned-strings branch from 3e5493a to 8e3a621 Compare September 4, 2026 10:22

OriolMunoz-da changed the title ~~[wip] Oriol/interned strings~~ Implement String interning store Sep 4, 2026

OriolMunoz-da commented Sep 4, 2026

View reviewed changes

OriolMunoz-da marked this pull request as ready for review September 4, 2026 10:28

OriolMunoz-da requested review from rautenrieth-da and ray-roestenburg-da September 4, 2026 10:28

OriolMunoz-da commented Sep 4, 2026

View reviewed changes

ray-roestenburg-da reviewed Sep 4, 2026

View reviewed changes

OriolMunoz-da mentioned this pull request Sep 4, 2026

Write per snapshot table #6742

Draft

ray-roestenburg-da self-requested a review September 4, 2026 11:28

ray-roestenburg-da approved these changes Sep 4, 2026

View reviewed changes

rautenrieth-da approved these changes Sep 4, 2026

View reviewed changes

OriolMunoz-da added 12 commits September 9, 2026 15:50

Add InternedStringStore

4208483

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

wire it up [ci]

927024b

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

lints [ci]

7f911e0

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

[ci] fix query and truncate

131a323

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

[ci] fmt

822e0e1

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

[ci] warmup the cache

a417f7a

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

[ci] also intern package name and choice name

54c960e

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

[ci] updateTestConfigForParallelRuns

d8b5012

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

[ci] change todo

ec73ad6

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

[ci] increase connection pool size

61194b7

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

[ci] add copyright, remvoe truncate

10fc90a

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

[ci] sigh

9f8259e

Signed-off-by: Oriol Muñoz <oriol.munoz@digitalasset.com>

OriolMunoz-da force-pushed the oriol/interned-strings branch from 31f2df4 to 9f8259e Compare September 9, 2026 13:50

OriolMunoz-da merged commit 411cfcd into main Sep 9, 2026
115 of 118 checks passed

OriolMunoz-da deleted the oriol/interned-strings branch September 9, 2026 14:55

Uh oh!

Conversation

OriolMunoz-da commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OriolMunoz-da Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OriolMunoz-da Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ray-roestenburg-da Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ray-roestenburg-da Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ray-roestenburg-da left a comment

Choose a reason for hiding this comment

Uh oh!

rautenrieth-da left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

OriolMunoz-da commented Sep 3, 2026 •

edited

Loading

OriolMunoz-da Sep 4, 2026 •

edited

Loading

OriolMunoz-da Sep 4, 2026 •

edited

Loading

ray-roestenburg-da Sep 4, 2026 •

edited

Loading

ray-roestenburg-da Sep 4, 2026 •

edited

Loading