Skip to content

Fix flaky MaestroWorkflowDaoTest properties updates - #240

Merged
akashdw merged 1 commit into
mainfrom
ad/fix-flaky-workflow-properties-dao-test
Aug 26, 2026
Merged

akashdw merged 1 commit into
mainfrom
ad/fix-flaky-workflow-properties-dao-test

Conversation

@akashdw

@akashdw akashdw commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes (Please run ./gradlew build --write-locks to refresh dependencies)
  • Other (please describe):

Changes in this PR

MaestroWorkflowDaoTest fails intermittently with

duplicate key value violates unique constraint "maestro_workflow_properties_pkey"
Detail: Key (workflow_id, create_time)=(sample-active-wf-with-triggers, 1787704031240) already exists.

maestro_workflow_properties is keyed on (workflow_id, create_time) and updateWorkflowProperties fills create_time from System.currentTimeMillis(), so two updates on the same workflow inside one millisecond collide. Four tests call it repeatedly on one workflow with almost nothing in between, and on a fast machine two of the calls share a millisecond.

Those calls now go through a test helper that sleeps a millisecond first. No production change.

}

/**
* maestro_workflow_properties is keyed on (workflow_id, create_time) and the dao takes

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maestro_workflow_properties is keyed on (workflow_id, create_time) - pretty unfortunate, would have liked if it was a monotonic key instead but I know that's out of scope for this PR

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

agreed, the timestamp in the key is the real problem and the sleep only hides it in tests. I'll think more on adding the monotonic version column in a backward compatible way.

@akashdw
akashdw merged commit e3d0571 into main Aug 26, 2026
1 check passed
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