Samza runner support for non unique stateId across multiple ParDos

[seungjin-an]

Samza runner support for non-unique stateIds across multiple ParDos
(1) Do not touch existing RocksDB Store Id with stateId used only by one ParDo
(2) Add a prescan logic in SamzaRunner to traverse Beam pipeline topologically and identify non-unique stateIds.
(3) Using the set of non-unique stateIds, introduce a new mapping of user-provided stateIds to store ID

// stateId used in a single ParDo continue to use the same stateId as storeId
.apply("First stateful ParDo", ParDo.of(fn));
→ “foo"

// stateId used in multiple ParDos use a new mapping to store Id with PTransform name
.apply("Second stateful ParDo", ParDo.of(fn2));
→ “foo-Second_stateful_ParDo”

(4) Re-enable ParDoTest in :runners:samza build.gradle

---

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

• Choose reviewer(s) and mention them in a comment (R: @username).
• Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
• Update CHANGES.md with noteworthy changes.
• If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

[dxichen]

Minor style/readability comments, thanks for the diff!

[xinyuiscool]

Two main issues:

1. no rewriting of configs, instead, do a prescan of pipeline and build the non-unique state id set
2. the changes in the SamzaStateInternals are not needed if you make the stores map still follow the same definition of {stateId -> store}. The state internals is generated per PTransform so it will not have dups here.

[seungjin-an]

Two main issues:

1. no rewriting of configs, instead, do a prescan of pipeline and build the non-unique state id set
2. the changes in the SamzaStateInternals are not needed if you make the stores map still follow the same definition of {stateId -> store}. The state internals is generated per PTransform so it will not have dups here.

Thank you so much for taking the time reviewing, providing suggestions on the pre-scan/ better naming, and fixing my understanding in SamzaStoreInternals. I have addressed all the comments and please have another look.

[xinyuiscool]

Thanks for fixing the problem for #1 in my previous comments. However, seems some of the necessary changes are also reverted for #2. I expect passing a mapping {stateId -> storeId} (currently it's a set of stateId) to the SamzaStoreStateInternals.createStateInternalsFactory(). Then createStateInternalsFactory() can use that information to create the stores map. Somehow this logic is missing. It was present in your previous code. Please take a look and run ./gradlew :runners:samza:build to verify the tests pass.

[seungjin-an]

Thank you @xinyuiscool for the review again. SamzaStoreStateInternals logic has been included and verified the changes with all the tests passing from ./gradlew :runners:samza:build

[xinyuiscool]

Overall looks great! Have some minor comments about consolidating some duplicate logic into a single class.

[seungjin-an]

Overall looks great! Have some minor comments about consolidating some duplicate logic into a single class.

@xinyuiscool Awesome, I've addressed all the comments above. Please review upon your availability! Thank you.

[xinyuiscool]

LGTM!

[xinyuiscool]

Run Java PreCommit