Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Order objects used for template data #7031

Merged
merged 3 commits into from Oct 18, 2019
Merged

Order objects used for template data #7031

merged 3 commits into from Oct 18, 2019

Conversation

@raskchanky
Copy link
Member

raskchanky commented Oct 11, 2019

Previously, we used HashMap to hold key/value pairs for template data. Unfortunately, HashMap iterates over its data in a non-deterministic order. This can cause problems when the checksum of the rendered file has changed, because the supervisor will reload the service.

We now use BTreeMap to hold these key/value pairs. It has a compatible API with HashMap and it iterates over its data in sorted order, which should fix this issue.

Fixes #6713

@chef-expeditor

This comment has been minimized.

Copy link

chef-expeditor bot commented Oct 11, 2019

Hello raskchanky! Thanks for the pull request!

Here is what will happen next:

  1. Your PR will be reviewed by the maintainers.
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

@raskchanky raskchanky force-pushed the jb/order-template-data branch from 39466cb to e4e296f Oct 11, 2019
@krasnow krasnow added this to the Habitat NAME (Iteration 6) milestone Oct 14, 2019
@@ -142,104 +240,6 @@ fn is_dest_on_path(dest_dir: &Path) -> bool {
}
}

struct Binlink {

This comment has been minimized.

Copy link
@christophermaier

christophermaier Oct 14, 2019

Contributor

Why did this chunk move?

This comment has been minimized.

Copy link
@raskchanky

raskchanky Oct 14, 2019

Author Member

Because I found it confusing that it was defined at the bottom of the file. Typically our structs and such are defined before they're used, which I get is not a requirement. I do find it makes the file more readable though.

Copy link
Contributor

christophermaier left a comment

Looks like you need to rebase, but other than that 👍

raskchanky added 2 commits Oct 11, 2019
The goal here is to provide predictable ordering of binds when they're
rendered to a template. BTreeMap iterates over its keys in sorted order,
whereas HashMap iterates over its keys in a non-deterministic order.
Unfortunately, changing the type in that one place bubbled out into many
different places across many different crates.

I ultimately decided to propagate the change all the way through to the
edges, which ended up being the launcher protocol crate. At that edge,
we convert from BTreeMap to HashMap and back, as the protobuf map data
type deserializes to a rust HashMap.

Signed-off-by: Josh Black <raskchanky@gmail.com>
Signed-off-by: Josh Black <raskchanky@gmail.com>
@raskchanky raskchanky force-pushed the jb/order-template-data branch from e4e296f to 20a12f1 Oct 16, 2019
Signed-off-by: Josh Black <raskchanky@gmail.com>
@raskchanky raskchanky force-pushed the jb/order-template-data branch from 7861575 to b5e9af2 Oct 17, 2019
@raskchanky raskchanky merged commit 40cca43 into master Oct 18, 2019
5 checks passed
5 checks passed
DCO This commit has a DCO Signed-off-by
Details
buildkite/habitat-sh-habitat-master-verify Build #3771 passed (38 minutes, 15 seconds)
Details
buildkite/habitat-sh-habitat-master-website Build #855 passed (52 seconds)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
expeditor/config-validation Validated your Expeditor config file
Details
@raskchanky raskchanky deleted the jb/order-template-data branch Oct 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.