[Fix #1691] MVStore additional objects persistence by fjtirado · Pull Request #1715 · open-workflow-specification/sdk-java

fjtirado · 2026-09-23T15:50:05Z

Copilot

Copilot review overview

🟡 Changes recommended

Metadata persistence loss, stale blob accumulation, thread-safety regression, and the misspelled public API constant must be addressed.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1 · 1

Open (3)

Persist metadata in instance and retry snapshots · New
Restored metadata map loses concurrent map guarantees · New
Fix typo in public TRESHOLD constant · New

What changed in this PR

Adds MVStore metadata persistence with versioned serialization, blob storage, and optional hashing.

Changes:

Persists and restores workflow metadata.
Adds configurable integer/MD5 hashing and blob storage.
Expands persistence and marshalling tests.

File	Summary
`impl/persistence/tests/src/main/java/io/serverlessworkflow/impl/persistence/test/AbstractHandlerPersistenceTest.java`	Adds metadata persistence assertions.
`impl/persistence/mvstore/src/test/java/io/serverlessworkflow/impl/persistence/mvstore/MVStorePersistenceStoreTest.java`	Configures hash factories.
`impl/persistence/mvstore/src/test/java/io/serverlessworkflow/impl/persistence/mvstore/MD5MVStorePersistenceTest.java`	Adds MD5 persistence coverage.
`impl/persistence/mvstore/src/test/java/io/serverlessworkflow/impl/persistence/mvstore/HashingMVStorePersistenceTest.java`	Adds integer-hash persistence coverage.
`impl/persistence/mvstore/src/main/java/io/serverlessworkflow/impl/persistence/mvstore/MVStoreTransaction.java`	Adds blob-map handling.
`impl/persistence/mvstore/src/main/java/io/serverlessworkflow/impl/persistence/mvstore/MVStorePersistenceStore.java`	Wires hashing and locking.
`impl/persistence/bigmap/src/main/java/io/serverlessworkflow/impl/persistence/bigmap/BytesMapInstanceTransaction.java`	Serializes metadata and hashed objects; metadata can be lost before completion or on retries, and stale blobs can accumulate.
`impl/persistence/bigmap/src/main/java/io/serverlessworkflow/impl/persistence/bigmap/BigMapInstanceTransaction.java`	Passes instance IDs during task decoding.
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/WorkflowPersistenceInstance.java`	Restores metadata.
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/hashing/MD5HashItem.java`	Adds MD5 hash representation; the public threshold constant is misspelled.
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/hashing/IntegerHashItem.java`	Adds integer hash representation.
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/hashing/HashItem.java`	Defines the hash-item contract.
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/hashing/HashFactory.java`	Defines hash creation and decoding.
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/hashing/DisabledHashFactory.java`	Provides disabled hashing.
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/hashing/AbstractHashFactory.java`	Decodes supported hash types.
`impl/persistence/api/src/main/java/io/serverlessworkflow/impl/persistence/CompletedTaskInfo.java`	Carries restored metadata.
`impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowMutableInstance.java`	Supports metadata restoration; restored values can replace the concurrent map with a non-thread-safe map.
`impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/WorkflowOutputBuffer.java`	Adds raw-byte writing.
`impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/MarshallingUtils.java`	Adds generic object decoding.
`impl/core/src/main/java/io/serverlessworkflow/impl/marshaller/DefaultOutputBuffer.java`	Implements raw-byte writing.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved concurrency, persistence, and public API compatibility issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 4 · 1

Open (5)

New abstract buffer method breaks custom implementations · New
Changed abstract persistence hook breaks external subclasses · New
Concurrent transactions can overwrite colliding hash blobs · New
Persist metadata in instance and retry snapshots
Restored metadata map loses concurrent map guarantees

Resolved since last review (1)

Fix typo in public TRESHOLD constant

Copilot

Copilot review overview

🟡 Changes recommended

Four moderate issues affecting metadata restoration and MVStore transaction handling remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3

Open (3)

Retry restore drops persisted task metadata · New
Restored instance loses metadata without a task record · New
Callback opens map from wrong transaction · New

Resolved since last review (5)

Concurrent transactions can overwrite colliding hash blobs
Changed abstract persistence hook breaks external subclasses
New abstract buffer method breaks custom implementations
Persist metadata in instance and retry snapshots
Restored metadata map loses concurrent map guarantees

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved critical issues affect hash decoding, metadata serialization consistency, and blob transaction atomicity.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 · 1

Open (4)

Decoder rejects custom hash item IDs · New
Concurrent metadata changes corrupt encoded entry count · New
Hash transaction does not coordinate blob map updates · New
Callback opens map from wrong transaction

Resolved since last review (2)

Restored instance loses metadata without a task record
Retry restore drops persisted task metadata

Copilot

Copilot review overview

🟡 Changes recommended

Four moderate review issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 · 2

Open (4)

Concurrent metadata changes corrupt encoded entry count
Decoder rejects custom hash item IDs
Blob writes can leave orphaned entries after transaction rollback · New
Avoid committing rolled-back transactions · New

Resolved since last review (2)

Hash transaction does not coordinate blob map updates
Callback opens map from wrong transaction

Copilot

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review effort: Lite
Findings: None

Resolved since last review (1)

Unsupported hash markers corrupt hashed value reads

Copilot

Copilot review overview

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review effort: Lite
Findings: 1

Open (1)

Hashing metadata test omits close and reopen persistence verification

Resolved since last review (1)

Rollback leaves transient mapping entries and creates index gaps

…g generated index Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

Copilot

Copilot review overview

🔵 Needs a closer look

The blob-map key format is unsafe for keys containing colons, and retry metadata coverage should be strengthened.

Review effort: Lite
Findings: 1

Open (1)

Hashing metadata test omits close and reopen persistence verification

Copilot

Copilot review overview

🔵 Needs a closer look

Unresolved persistence issues affect hashed-key handling and metadata recovery during status updates.

Review effort: Lite
Findings: None

Resolved since last review (1)

Hashing metadata test omits close and reopen persistence verification

fjtirado · 2026-09-25T14:40:37Z

One of the open issues between the AI and myself is to bound memory usage, Ill do that in a different PR where the static ConcurrentMap cache is replaced by an LRUCache.java (based on ConcurrentMap)

But I prefer a different PR because if the AI is really suffocating with this one, If I add the LRUCAche now, its going to allucinate.

Copilot

Copilot review overview

🟡 Changes recommended

Composite blob-map keys can become unreadable when custom hash keys contain :.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Prevent colon-containing keys from corrupting persisted hash entries · New

…lity Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

Copilot

Copilot review overview

🟡 Changes recommended

Two moderate lifecycle and key-encoding issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Colon-delimited blob keys break custom key and index values · New

Resolved since last review (1)

Prevent colon-containing keys from corrupting persisted hash entries

Copilot

Copilot review overview

🔵 Needs a closer look

The broad persistence, hashing, serialization, and compatibility changes require final human review.

Review effort: Lite
Findings: None

Resolved since last review (1)

Colon-delimited blob keys break custom key and index values

fjtirado requested review from gmunozfe and ricardozanini and a lite review from Copilot September 23, 2026 15:50

Copilot started reviewing on behalf of fjtirado September 23, 2026 15:50 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 23, 2026 15:54

fjtirado force-pushed the Fix_#1691 branch from fe3218d to c45d18e Compare September 23, 2026 15:54

Copilot started reviewing on behalf of fjtirado September 23, 2026 15:54 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 23, 2026 16:25

fjtirado force-pushed the Fix_#1691 branch from c45d18e to 9827c67 Compare September 23, 2026 16:25

Copilot started reviewing on behalf of fjtirado September 23, 2026 16:25 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 23, 2026 16:33

fjtirado force-pushed the Fix_#1691 branch from 9827c67 to 52ddb4f Compare September 23, 2026 16:33

Copilot started reviewing on behalf of fjtirado September 23, 2026 16:33 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 23, 2026 16:49

fjtirado force-pushed the Fix_#1691 branch from 52ddb4f to 1eccf64 Compare September 23, 2026 16:49

Copilot started reviewing on behalf of fjtirado September 23, 2026 16:49 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread ...src/main/java/io/serverlessworkflow/impl/persistence/bigmap/BytesMapInstanceTransaction.java Outdated

Comment thread ...mvstore/src/main/java/io/serverlessworkflow/impl/persistence/mvstore/MVStoreTransaction.java Outdated

Copilot AI review requested due to automatic review settings September 23, 2026 17:05

fjtirado force-pushed the Fix_#1691 branch from 1eccf64 to 0937a8b Compare September 23, 2026 17:05

Copilot started reviewing on behalf of fjtirado September 23, 2026 17:05 View session

This comment was marked as outdated.

Sign in to view

Copilot AI review requested due to automatic review settings September 23, 2026 17:15

fjtirado force-pushed the Fix_#1691 branch from 0937a8b to 7018965 Compare September 23, 2026 17:15

Copilot started reviewing on behalf of fjtirado September 23, 2026 17:16 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 23, 2026 17:32

fjtirado force-pushed the Fix_#1691 branch 2 times, most recently from 6705207 to 7b1b817 Compare September 25, 2026 12:28

Copilot started reviewing on behalf of fjtirado September 25, 2026 12:28 View session

fjtirado marked this pull request as draft September 25, 2026 12:30

Copilot AI reviewed Sep 25, 2026

View reviewed changes

fjtirado force-pushed the Fix_#1691 branch from 7b1b817 to 7e72b91 Compare September 25, 2026 13:27

[Fix open-workflow-specification#1691] Moving from int index to strin…

a1c3cad

…g generated index Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

fjtirado force-pushed the Fix_#1691 branch from 7e72b91 to a1c3cad Compare September 25, 2026 13:34

fjtirado marked this pull request as ready for review September 25, 2026 14:04

Copilot AI review requested due to automatic review settings September 25, 2026 14:05

Copilot started reviewing on behalf of fjtirado September 25, 2026 14:05 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 25, 2026 14:11

fjtirado force-pushed the Fix_#1691 branch from e9db6d3 to fc8a180 Compare September 25, 2026 14:11

Copilot started reviewing on behalf of fjtirado September 25, 2026 14:12 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

fjtirado requested a lite review from Copilot September 25, 2026 15:13

Copilot started reviewing on behalf of fjtirado September 25, 2026 15:13 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread ...src/main/java/io/serverlessworkflow/impl/persistence/bigmap/BytesMapInstanceTransaction.java Outdated

fjtirado changed the title ~~[Fix #1691] Add metadata to mvstore~~ [Fix #1691] MV stora additional object persistence Sep 25, 2026

[Fix open-workflow-specification#1691] Use Ulid for improving versati…

4c90bca

…lity Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

fjtirado force-pushed the Fix_#1691 branch from fc8a180 to 4c90bca Compare September 25, 2026 15:40

Copilot AI review requested due to automatic review settings September 25, 2026 15:40

Copilot started reviewing on behalf of fjtirado September 25, 2026 15:40 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread ...src/main/java/io/serverlessworkflow/impl/persistence/bigmap/BytesMapInstanceTransaction.java

fjtirado changed the title ~~[Fix #1691] MV stora additional object persistence~~ [Fix #1691] MVStore additional objects persistence Sep 25, 2026

fjtirado requested a lite review from Copilot September 25, 2026 15:55

Copilot started reviewing on behalf of fjtirado September 25, 2026 15:55 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Conversation

fjtirado commented Sep 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

fjtirado commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

fjtirado commented Sep 25, 2026 •

edited

Loading