Aquarium and pH

[philipcmonk]

A lot happening here. I've sent out a usage document, which should probably become public documentation at some point.

Main changes:

• Aquarium created
• pH created
• Clay queues incoming changes so that no more than one change happens in a single event
• Various small sequencing changes in vanes, to improve robustness
• Clay stores the rift number that we had for the foreign ship with its desks. We should use this information for individual breaches, but this doesn't attempt to solve that.

[joemfb]

Minor cleanup request:

[joemfb]

@philipcmonk, how long do some of these basic tests task? It'd be nice to wire something up for CI, as an example for future testing efforts if nothing else.

[philipcmonk]

Haven't timed it, but something like 5-10 minutes on my machine. It's significantly faster if you don't start collections, acme, dns, and hall. If it's easy to wire up to CI, I'd say we should do it. CI is changing in cc-release, so we also could just hook it up to CI there.

[belisarius222]

A little comment explaining that this ignores effects other than %restore or %send might be helpful.

[belisarius222]

Does this need to be the same /0v1n.2m9vh as in the ames handler? Maybe that constant should be split off into sur/aquarium/hoon.

[joemfb]

That value is u3A->sen, the "instance number" in vere. It's set to (scot %uv (mug now)) on boot/restart. I think the idea is to ensure there's a new duct corresponding to the new process, but it's not used for effect routing (or for injecting %wake events). I don't think it's needed here. It might not be needed at all, but that's a larger conversation.

[philipcmonk]

I just cargo-culted that. The right answer is probably for aqua to keep track of that, change it on restart, and insert it into each of these wire as necessary. For now, I've just copied the same string everywhere.

[belisarius222]

Do :fleet-snaps and :piers refer to the same ships? If so, it might be cleaner to have :fleet-snaps be just a (map term (set ship)) so there's less risk of the two getting out of sync.

[philipcmonk]

fleet-snaps is a set of snapshots of the state of the ships. It's intended to freeze a fleet so you can restore back to that point later on. This is how pH can cache e.g. "start ~bud" so that you only have to do it once rather than once per test.

[belisarius222]

I had some minor comments, but other than the eth-manage address change, I approve this for merging.

[belisarius222]

ovo refers to an event

[joemfb]

You're both wrong: ovo is the conventional name for an +ovum, which is (pair wire card). An "event" is (pair @da ovum).

[belisarius222]

Huh, looks like you turned off the culling if our own ship is the one subscribed. Does that still work for web stuff?

[philipcmonk]

I tried a bunch of stuff to reduce the number of subscription updates in the queues between aqua/ph and aqua-*, and I couldn't come up with anything that would work reliably.

I'm not sure how web stuff uses this? Is that the only way we clean up subscriptions created by web pages?

[joemfb]

@philipcmonk, i think it's worth adding to CI. It's not particularly complicated to add now, and then re-add once arvo/next converges with vere/cc-release. Duplicating the |mass implementation (here: https://github.com/urbit/arvo/blob/next/.travis/test.js#L77) gets you most of the way, you just need to scrape for an error condition (a la https://github.com/urbit/runner-js/blob/master/lib/actions.js#L26). I can help if runner-js gives you trouble.

[philipcmonk]

Added it to CI. Now CI takes ~25 minutes. Looks like the old builds took between 8 and 17 minutes (not sure what causes them to be longer; maybe changes to /sys?).

[ohAitch]

Changes to sys/ definitely *should* take longer, as they build a new pill; are there many that are not close to 8 or close to 17 minutes?

…

On Fri, Mar 29, 2019 at 4:59 PM Philip Monk ***@***.***> wrote: Added it to CI. Now CI takes ~25 minutes. Looks like the old builds took between 8 and 17 minutes (not sure what causes them to be longer; maybe changes to /sys?). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1120 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABxXhqZsUifobA8vJ5fGvKYRqv_X7f_Iks5vbqjggaJpZM4cEoU2> .

[ohAitch]

Also, would it be possible to add custom folds for individual test phases showing more granular timings? Even 8 minutes is rather a lot.

(5 of those is the actual test script, 8 in the pill case; it looks like we're maybe building the pill twice??)

[joemfb]

@ohAitch, we have to build a second pill to correctly capture userspace changes (which are now stored in the pill).

I think the right approach is to a) split the different test types across different CI jobs and b) cache and promote build artifacts more efficiently. The right place will be on top of cc-release, which now uses nix and make as the build system, and urb.py as the test runner. The right time will be when somebody is free to work on it.

[ohAitch]

Hm, I don't see it being built at all in the 8 minute cases though, is this a master vs cc-release difference? (And even if you have to add userspace, recompiling everything from scratch seems superfluous given you could instead just mutate the "userspace files" event at the end of the pill) Agree that the Travis build matrix seems useful here, if most of the test time is in fact straightforwardly parallelizable.

…

On Friday, 29 March 2019, Joe Bryan ***@***.***> wrote: ***@***.**** approved this pull request. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1120 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABxXho-G1P51CtKs0xv6Dfxnh-E_jwLAks5vbuccgaJpZM4cEoU2> .

[joemfb]

It's still only built on changes to sys, but when it's built it's build twice twice: first to stage the kernel changes, second to make the complete up-to-date pill that will be uploaded. This is of course stupid and inefficient, but was the easiest way to make the new pills not wrong. My grand plan is to let the pain of these slow builds increase until someone (maybe me!) snaps and makes it fast.

[ohAitch]

In that case it is certainly correct to merge the 25-minute version B)

…

On Friday, 29 March 2019, Joe Bryan ***@***.***> wrote: It's still only built on changes to sys, but when it's built it's build twice twice: first to stage the kernel changes, second to make the complete up-to-date pill that will be uploaded. This is of course stupid and inefficient, but was the easiest way to make the new pills not wrong. My grand plan is to let the pain of these slow builds increase until someone (maybe me!) snaps and makes it fast. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1120 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABxXhoXFduH_mLPkEjTSQwJ9zp9qnLORks5vbupugaJpZM4cEoU2> .