feat: Data partitions for snapshot and durable state #515
Conversation
patriknw
force-pushed
the
wip-more-data-partitions-patriknw
branch
from
73af6fe to
fdfd25a
Compare
|
Ready for review, I think it should cover everything for snapshot and durable state. |
patriknw
force-pushed
the
wip-more-data-partitions-patriknw
branch
from
fdfd25a to
9f379a7
Compare
| * the table name is without suffix. | ||
| */ | ||
| def snapshotTableWithSchema(slice: Int): String = | ||
| resolveTableName(snapshotsTableWithSchema, slice) |
There was a problem hiding this comment.
This is accidentally used in PostgresSnapshotDao, can we make the name factory private and have a public method that uses the table to avoid such mistakes?
There was a problem hiding this comment.
What do you mean? This is the new method, which should always be used. The old snapshotsTableWithSchema without slice parameter could become private, but I didn't want to break bin compat (@InternalStableApi), but maybe worth doing that?
There was a problem hiding this comment.
I deprecated the old tableWithSchema methods
There was a problem hiding this comment.
Sorry, maybe I was unclear, what I meant is that the consuming code calls this, which generates a new string for every call instead of using the allSnapshotTablesWithSchema lookup table defined below. Probably not a big thing but seems wasteful when we anyway have the lookup table.
There was a problem hiding this comment.
Ah, I see now the lookup table is the reverse, name to slice
There was a problem hiding this comment.
I see what you mean, but allSnapshotTablesWithSchema isn't a lookup for each of the 1024 slices. It contains one entry per data partition, where the slice is the lower slice for that data partition.
E.g. snapshotTableWithSchema(slice = 17) can't be found easily in allSnapshotTablesWithSchema.
To make it a full lookup table it would have to contain 1024 entries, with the slice as key. Might be possible with some other structure such as binary search but not sure that is worth it. Then I'm more worried about all string construction of the full sql statements that now happen on each use. We could introduce a cache for them, and that would also solve this because the table names are always used within such sql.
allSnapshotTablesWithSchema was mostly added because in tests we delete all tables before running.
| * The durable state table and schema name with data partition suffix for the given slice. When number-of-partitions | ||
| * is 1 the table name is without suffix. | ||
| */ | ||
| def durableStateTableWithSchema(slice: Int): String = |
There was a problem hiding this comment.
Also used all over the place instead of the prepared lookup table.
There was a problem hiding this comment.
Similar comment here. With lookup table, do you mean _durableStateTableByEntityType. For Durable State it was already possible to configure separate table per entity type, and the accessor for that is
def getDurableStateTableWithSchema(entityType: String, slice: Int)
but I think we also need this one for the default durable_state table
core/src/main/scala/akka/persistence/r2dbc/internal/postgres/PostgresDurableStateDao.scala
Outdated
Show resolved
Hide resolved
patriknw
force-pushed
the
wip-more-data-partitions-patriknw
branch
2 times, most recently
from
f6b13dd to
3ec6548
Compare
patriknw
force-pushed
the
wip-more-data-partitions-patriknw
branch
from
b8342d3 to
14c2d67
Compare
On top of #508 , and not complete yet