-
Notifications
You must be signed in to change notification settings - Fork 26
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
various: %egg-any import support #3830
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Support %egg-any imports in most of our important agents. To keep the logic simple, we only directly import locally-hosted data and configurations, and depend on group re-joining for everything else. In order for re-joins of locally-hosted groups to work, the groups agent must wait with its imports until channels-server has run its import, because the join causes channels to make subscriptions to channels-server. Not yet properly tested.
Instead of running a %group-join, just add the group into state, and join all its channels with +join-channels.
Confirmed correct.
Primarily, resolve an implementation bug where we weren't accounting for the proper shape of the +on-save vase.
Remove our own state, transparentize our own subscriptions, letting the inner agent resolve the import as if we weren't there.
Previously, if we tried to join a channel we already had state for, we would crash. Here, we do a check on our subs, reestablish them if needed, but otherwise no-op. This lets us import channel state from backups without it interfering with subsequent group rejoins. It also speeds up the rejoins: we simply start fetching updates again from where we the imported state left off.
By adding the entry into the bunt, instead of relying on +on-init or migration logic to fill it in, we get stronger guarantees about it always being there.
We do re-establish the subscriptions, so we should hear any changes to the group we missed while we were down.
We apply the $pimp pattern to other agents, so that we can enforce import order of activity -> channels-server -> channels -> groups. We care about the order being that way, because channels might subscribe to channels-server, and groups will tell channels where to resubscribe, but those subscriptions should key off the data we already have instead of starting from a clean slate.
Adds a minimally viable %noun %group-wake poke that can be used to make a group member re-check all its subscriptions for the group and its channels. This way, members automatically start receiving updates for a rescued group again. Depends on the channels agent change from faeb812.
When importing from a backup, send all the subscribers we remember a poke so that they can try re-establishing their subscriptions. Doing this here also means that in the groups wake case, we can stop doing this for all channels in the group, which might not be accurated for mixed group-host channel-host scenarios.
arthyn
reviewed
Aug 12, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall changes look good, a few small changes are warranted, and I might need you to walk me through the negotiate diff.
We need these for floor and bump time.
arthyn
approved these changes
Aug 14, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds support for restoring agent state from
$egg-any:gall
backups. (That's the backup format phoenix supports.)This restores both locally-hosted and remote data. We do our best to re-establish subscriptions based on the data we have, generally picking up where we left off. It restores state for DMs (chat), groups (groups, channels-server, channels, activity) and public profiles (profile). Contacts data (at least our own profile) should probably support this, but it outside of this repo's scope.
I tested this pretty extensively prior to the addition of restoring non-locally-hosted data, and that seemed to work well. After the latest batch of changes, I'll want to do another serious round of testing.
(
Specifically, I may have introduces some additional ordering concerns that I'll need to address. I suspect we want to do both channels and channels-server before groups now, and maybe activity before all of those...Done. And, need to make sure subscriptions get picked up at the right places under all circumstances. ...To be continued.)One missing piece here is actually on the "everyone else" side of this code: if a ship that hosts a group breaches, comes back, then re-imports the group, those subscriptions don't get re-established automatically. We may or may not be able to solve this from the group host side. (This may be more complicated for the "hosting a channel in a group I don't host myself" cases, too.) So, some pending work here, but may not be considered strictly blocking.Done.Realizing now that this was tested only on fully-joined states, not on weird in-between "trying to join the group" states or stuff like that, or on invites for that matter. Those are probably of secondary concern, but we should know what the behavior there is at least.
Fixes TLON-2192