feat(hydro_lang): multi-versioning support in the simulator#2945
feat(hydro_lang): multi-versioning support in the simulator#2945luckyworkama wants to merge 1 commit into
Conversation
Deploying hydro with
|
| Latest commit: |
2ebc4d6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f290b647.hydroflow.pages.dev |
| Branch Preview URL: | https://push-mxwwtmrsnmzm.hydroflow.pages.dev |
8f60b45 to
653854f
Compare
cafb97a to
14e48b2
Compare
shadaj
left a comment
There was a problem hiding this comment.
First pass, have not reviewed the send half and recv half bits yet.
|
hmm, deriving the version from an existing cluster seems a bit weird. In the sim_multi_version_differing_topology test, it creates 3 versions, gossip_v1, gossip_v2 and gossip_v3 (which includes a new Logger cluster). The issue is that the Logger cluster is internally going to have a v0 stamp on it, while gossip_v3 will have a v3 stamp on it. This wasn't the case with the flow-wide versioning api before. I think it is not actually an issue? It does feel kinda weird tho. @shadaj |
Co-authored-by: Infinity <infinity@hydro.run> Address shadaj's review comments on PR #2945 (multi-versioning sim) hydro_lang/src/compile/ir/mod.rs: - Change the two `panic!`s in the non-simulation builder's `create_versioned_network_fork` / `create_versioned_network` stubs to `unreachable!`, since these nodes are only produced by the simulator merge pass. hydro_lang/src/compile/builder.rs: - Fix `next_version` to compute the next version number from the highest version already assigned to any location in the correspondence group, rather than from the `from` cluster directly. This avoids version-number collisions when `next_version` is called more than once on the same source cluster. hydro_test/src/distributed/versioning.rs: - Replace the three near-identical `gossip_server_v1/v2/v3` functions with a single `gossip_server_versioned` that takes a `GossipVersion` enum and branches internally (matching the approach real versioned code would take), updating all call sites. - Remove the redundant `sim_multi_version_basic_echo_fuzz` test, since Bolero guarantees fuzz explores a subset of exhaustive. - Add `sim_multi_version_two_clusters_differing_version_counts`, exercising two independent clusters with different numbers of versions (2 and 3) to guard against bugs when version counts differ across clusters. Co-authored-by: Infinity 🤖 <infinity@hydro.run>
14e48b2 to
b57af73
Compare
Co-authored-by: Infinity <infinity@hydro.run> Address shadaj's review comments on PR #2945 (multi-versioning sim) hydro_lang/src/compile/ir/mod.rs: - Change the two `panic!`s in the non-simulation builder's `create_versioned_network_fork` / `create_versioned_network` stubs to `unreachable!`, since these nodes are only produced by the simulator merge pass. hydro_lang/src/compile/builder.rs: - Fix `next_version` to compute the next version number from the highest version already assigned to any location in the correspondence group, rather than from the `from` cluster directly. This avoids version-number collisions when `next_version` is called more than once on the same source cluster. hydro_test/src/distributed/versioning.rs: - Replace the three near-identical `gossip_server_v1/v2/v3` functions with a single `gossip_server_versioned` that takes a `GossipVersion` enum and branches internally (matching the approach real versioned code would take), updating all call sites. - Remove the redundant `sim_multi_version_basic_echo_fuzz` test, since Bolero guarantees fuzz explores a subset of exhaustive. - Add `sim_multi_version_two_clusters_differing_version_counts`, exercising two independent clusters with different numbers of versions (2 and 3) to guard against bugs when version counts differ across clusters. Co-authored-by: Infinity 🤖 <infinity@hydro.run>
b57af73 to
2c82a18
Compare
Honestly, I think it's quite elegant! Basically each cluster can have a different number / set of versions, it's akin to having one dynamic versioning flag per-cluster. |
Co-authored-by: Infinity <infinity@hydro.run> Address shadaj's review comments on PR #2945 (multi-versioning sim) hydro_lang/src/compile/ir/mod.rs: - Change the two `panic!`s in the non-simulation builder's `create_versioned_network_fork` / `create_versioned_network` stubs to `unreachable!`, since these nodes are only produced by the simulator merge pass. hydro_lang/src/compile/builder.rs: - Fix `next_version` to compute the next version number from the highest version already assigned to any location in the correspondence group, rather than from the `from` cluster directly. This avoids version-number collisions when `next_version` is called more than once on the same source cluster. hydro_test/src/distributed/versioning.rs: - Replace the three near-identical `gossip_server_v1/v2/v3` functions with a single `gossip_server_versioned` that takes a `GossipVersion` enum and branches internally (matching the approach real versioned code would take), updating all call sites. - Remove the redundant `sim_multi_version_basic_echo_fuzz` test, since Bolero guarantees fuzz explores a subset of exhaustive. - Add `sim_multi_version_two_clusters_differing_version_counts`, exercising two independent clusters with different numbers of versions (2 and 3) to guard against bugs when version counts differ across clusters. Co-authored-by: Infinity 🤖 <infinity@hydro.run>
2c82a18 to
b450b9e
Compare
Co-authored-by: Infinity <infinity@hydro.run> Address shadaj's review comments on PR #2945 (multi-versioning sim) hydro_lang/src/compile/ir/mod.rs: - Change the two `panic!`s in the non-simulation builder's `create_versioned_network_fork` / `create_versioned_network` stubs to `unreachable!`, since these nodes are only produced by the simulator merge pass. hydro_lang/src/compile/builder.rs: - Fix `next_version` to compute the next version number from the highest version already assigned to any location in the correspondence group, rather than from the `from` cluster directly. This avoids version-number collisions when `next_version` is called more than once on the same source cluster. hydro_test/src/distributed/versioning.rs: - Replace the three near-identical `gossip_server_v1/v2/v3` functions with a single `gossip_server_versioned` that takes a `GossipVersion` enum and branches internally (matching the approach real versioned code would take), updating all call sites. - Remove the redundant `sim_multi_version_basic_echo_fuzz` test, since Bolero guarantees fuzz explores a subset of exhaustive. - Add `sim_multi_version_two_clusters_differing_version_counts`, exercising two independent clusters with different numbers of versions (2 and 3) to guard against bugs when version counts differ across clusters. Co-authored-by: Infinity 🤖 <infinity@hydro.run>
b450b9e to
2ebc4d6
Compare
No description provided.