Skip to content
This repository has been archived by the owner on Aug 14, 2019. It is now read-only.

hardens %ames initialization #1138

Merged
merged 4 commits into from
Apr 12, 2019
Merged

hardens %ames initialization #1138

merged 4 commits into from
Apr 12, 2019

Conversation

joemfb
Copy link
Member

@joemfb joemfb commented Apr 12, 2019

As discussed briefly in urbit/urbit#1236, %ames initialization is particularly fragile, and requires special accommodation by the runtime. This PR makes the handling of the ames process/boot event %barn more robust, at the expense of dropping (and therefore subsequently retrying) packets sent during the boot sequence.

@joemfb
Copy link
Member Author

joemfb commented Apr 12, 2019

Note: this PR is to %ames as #1130 was to %behn

@joemfb
Copy link
Member Author

joemfb commented Apr 12, 2019

/cc @philipcmonk, curious if you think this is a sufficient :aqua setup for now.

@philipcmonk
Copy link
Contributor

I don't like switching to just doing a "hi" test. In particular, I would like to make sure that children can boot -- that sort of thing breaks often, and it tests things like filesystem sync.

Do you know why it failed the previous time? It looks a little suspicious because one of the checks passed and the other failed, but I think they both run aqua. The one that failed hung on the last test, %child-sync, which boots a parent and child, then changes a file on the parent and checks to see if the file changed on the child.

If this suggests a non-determinism in the tests, then it might be worth squashing that. Perhaps a more deterministic view of time (ie a fully virtualized time) would solve it.

@joemfb
Copy link
Member Author

joemfb commented Apr 12, 2019

Since we added :aqua to the CI tests, it looks like "push" CI builds succeed and "pr" builds fail. I've double checked the CI config, but I don't see anything that's branching off of this condition.

I started fiddling with it here because I wanted to be sure I hadn't broken something. This change should only make %ames more robust during boot, but that's the kind of thing that can only be verified empirically (at least for now).

Do you think the tests should be augmented with timers to detect hangs?

@philipcmonk
Copy link
Contributor

philipcmonk commented Apr 12, 2019 via email

:_ fox
[[gad.fox [%give %send p.bon q.bon]] ~]
::
%pito
:_ fox(tim `p.bon)
%- flop
^- (list move)
:: XX should this be the unix duct (gad.fox)?
Copy link
Contributor

Choose a reason for hiding this comment

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

oof good question

I suspect we should make a specific constant duct whose sole purpose is this Ames-Behn interaction.
~[/ames-behn] might be ok. I think we'd want it not to have a // at the front, to disambiguate it from ducts that have Unix origins.

If we do change that, though, I think we should do it in a different PR so we can git-bisect later if we need to.

Copy link
Contributor

@belisarius222 belisarius222 left a comment

Choose a reason for hiding this comment

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

LGTM

@belisarius222
Copy link
Contributor

At some point, I'd really like to unify %barn and %born into one event. Doesn't need to happen now, though.

@joemfb joemfb merged commit cd13ddb into next Apr 12, 2019
@joemfb joemfb deleted the ames-init-safe branch April 12, 2019 20:45
@ixv ixv mentioned this pull request Apr 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants