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

feat(daemon): Add reverseLocate() and followLocatorNameChanges() #2195

Merged
merged 8 commits into from
Apr 17, 2024

Conversation

rekmarks
Copy link
Contributor

@rekmarks rekmarks commented Mar 30, 2024

Adds reverseLocate() and followLocatorNameChanges() to the daemon's directory. To match this new method, the directory's followChanges() and the pet store's underlying follow() are renamed to followNameChanges().

The directory's reverseLocate(locator) and followLocatorNameChanges(locator) are backed up by the pet store's reverseIdentify(id) and followIdNameChanges(id), respectively. The implementation of the former is very straightforward. The implementation of the latter required the introduction of a new pubsub topic, idChangesTopic, and a map from ids to idChangesTopic subscriptions (one for each id).

Since the pet store is unaware of whether ids exist, followIdNameChanges() will naively create a subscription that first publishes an empty array of names, and then any subsequent names that are added for that id. followLocatorNameChanges() inherits this property.

The current implementation suffers from the limitation that locator name change subscriptions cannot be ended. A follow-up will introduce the ability for subscribers to cancel their subscription. Locator subscriptions will also be a target for our future garbage collector, which will delete subscriptions for unreachable locators. Cancelling a locator should not end the subscription, because we consider the liveness of a value to be orthogonal to the subscriber's interest in changes to its names.

@rekmarks rekmarks force-pushed the rekmarks-multi-bimap-fixes branch from aec68a5 to 13bbec1 Compare March 30, 2024 03:00
@rekmarks rekmarks force-pushed the rekmarks-follow-reverse-locate branch from 1929957 to cb91afe Compare March 30, 2024 03:01
Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

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

So far, so good!

packages/daemon/src/directory.js Outdated Show resolved Hide resolved
packages/daemon/src/directory.js Outdated Show resolved Hide resolved
packages/daemon/src/locator.js Show resolved Hide resolved
@rekmarks rekmarks force-pushed the rekmarks-multi-bimap-fixes branch from 13bbec1 to 89524a3 Compare April 8, 2024 10:08
@rekmarks rekmarks force-pushed the rekmarks-follow-reverse-locate branch from cb91afe to 62a2efe Compare April 8, 2024 10:12
Base automatically changed from rekmarks-multi-bimap-fixes to master April 8, 2024 10:17
@rekmarks rekmarks force-pushed the rekmarks-follow-reverse-locate branch 2 times, most recently from b3b7c9c to 9784695 Compare April 11, 2024 23:32
@rekmarks rekmarks changed the title feat(daemon): Add reverseLocate() and followLocatorNames() feat(daemon): Add reverseLocate() and followLocatorNameChanges() Apr 11, 2024
@rekmarks rekmarks force-pushed the rekmarks-follow-reverse-locate branch from aa8be0b to 2b6fd73 Compare April 15, 2024 22:54
@rekmarks rekmarks requested a review from kriskowal April 15, 2024 23:02
@rekmarks rekmarks marked this pull request as ready for review April 15, 2024 23:02
Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

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

Worth considering whether we might just factor a light-client out of followLocatorNameChanges that is based on a sharable subscription to followNameChanges. That might put the tracking burden closer to where it ought to be and relax the need to perform timely identifier GC.

@@ -97,6 +97,30 @@ export const makeDirectoryMaker = ({
return formatLocator(id, formulaType);
};

/** @type {import('./types.js').EndoDirectory['reverseLocate']} */
const reverseLocate = async locator => {
Copy link
Member

Choose a reason for hiding this comment

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

Silly idea:

  • “locate” means “get locator for name”
  • “nominate” means “get name from locator”

packages/daemon/src/directory.js Outdated Show resolved Hide resolved
packages/daemon/src/pet-sitter.js Outdated Show resolved Hide resolved
Adds reverseLocate() to the directory. Also refactors locator utilities
for better coherence with their formula identifier equivalents. Adds
idFromLocator().
Adds an initial implementation of `followLocatorNameChanges()`, which
publishes the extant names for a specific id, if any. Tests are added
for the same.
Ensures that overwriting names during pet store renames publishes a
removal event. The removal event(s) will always be published first.
The behavior is tested both for overwrites and non-overwrites.
Replaces all references to "diff" in pubsub type names to "name change"
to match the names of the pubsub APIs (e.g. `followNameChanges`). Also
removes an extraneous `if` condition.
@rekmarks rekmarks force-pushed the rekmarks-follow-reverse-locate branch from 2b6fd73 to 37bb657 Compare April 17, 2024 00:06
@rekmarks rekmarks enabled auto-merge April 17, 2024 00:08
@rekmarks rekmarks merged commit d521532 into master Apr 17, 2024
18 checks passed
@rekmarks rekmarks deleted the rekmarks-follow-reverse-locate branch April 17, 2024 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants