Skip to content

Improve chasm Map deserialization/serialization logic#10270

Merged
awln-temporal merged 2 commits into
mainfrom
awln/chasm-map-improvements
May 18, 2026
Merged

Improve chasm Map deserialization/serialization logic#10270
awln-temporal merged 2 commits into
mainfrom
awln/chasm-map-improvements

Conversation

@awln-temporal
Copy link
Copy Markdown
Contributor

What changed?

Improve chasm Map deserialization/serialization logic

Why?

chasm.Map should not be added to DeletedNodes if never persisted. When deserializing, it should always be initialized to avoid excessive nil checks.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

@awln-temporal awln-temporal requested a review from a team as a code owner May 14, 2026 19:32
@awln-temporal awln-temporal requested a review from yycptt May 15, 2026 16:22
Copy link
Copy Markdown
Member

@yycptt yycptt left a comment

Choose a reason for hiding this comment

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

I think we need a support/test for the case where a node is created and then deleted in the same transaction but we performed a syncStructure in between which populates the DeletedNodes map (this happens when we process multiple pure tasks in the same transaction).

I think this issue applies to not just map node but component/data node as well. No real impact I believe, we are just deleting non-exist node. Let's see if we can first confirm the issue and if so we can try address it in a separate PR.

Comment thread chasm/tree.go
fmt.Errorf("node %s", n.nodeName))
}

if field.val.IsNil() || len(field.val.MapKeys()) == 0 {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we have a unit test for this case (not deleting newly created map)?

@awln-temporal
Copy link
Copy Markdown
Contributor Author

Will track node removal from DeletedNodes in a separate task

@awln-temporal awln-temporal force-pushed the awln/chasm-map-improvements branch from 0a595ed to 05cc96d Compare May 18, 2026 15:42
@awln-temporal awln-temporal merged commit 7b9ecf2 into main May 18, 2026
48 checks passed
@awln-temporal awln-temporal deleted the awln/chasm-map-improvements branch May 18, 2026 17:37
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.

2 participants