Skip to content

Commit

Permalink
Update implementation of streamChangesSinceV2 to use ForEachDelta
Browse files Browse the repository at this point in the history
Summary:
# Context

We are introducing EdenFS notifications to support scalable and ergonomic file system notifications for EdenFS mounts.

# This Diff

* Introduces a new, public `forEachDelta` method on `Journal` so that we can use it to properly traverse the journal for `streamChangesSinceV2`
* Replaces the usage of `accumulateRange` in `streamChangesSinceV2` to compute changes - still used to collect some summary data

# Next Steps

* Addresses the outstanding TODOs in `Journal` and `streamChangesSinceV2` - specifically, any range type information that was being computed and now is not and any information we are relying on from `acucumlateRange`
* Remove call to `accumulateRange`
* Add DType to journal entries.
* Testing, testing, testing

# Discussion Points

* `streamChangesSinceV2` is being executed inline. This seems OK as it makes no blocking calls and has an expected termination that does not take long to reeach. However, if we evolve this method to also support waiting for events (i.e. subscribe), then we would need to change that. Should we make those changes now?
* `streamChangesSinceV2` is returning changes from most recent to the requested postion - in other words in reverse order. To change this would require either: a) collecting all changes in memory then reversing them (what the previous version did) or b) locating the poistion in the journal and then traversing to the tail. Is this needed?

Differential Revision: D65906174

fbshipit-source-id: 5dc300872dd8d6b80f832a8b53725436935b7103
  • Loading branch information
jdelliot authored and facebook-github-bot committed Nov 16, 2024
1 parent e60f7f8 commit 10c03e8
Showing 1 changed file with 1 addition and 3 deletions.
4 changes: 1 addition & 3 deletions eden/fs/service/streamingeden.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,8 @@ struct Removed {
}

union SmallChangeNotification {
// @lint-ignore-every FBTHRIFTCOMPAT FBTHRIFTCOMPAT1 FBTHRIFTCOMPAT2
1: Added added;
// @lint-ignore-every FBTHRIFTCOMPAT FBTHRIFTCOMPAT1 FBTHRIFTCOMPAT2
2: Modified modified;
// @lint-ignore-every FBTHRIFTCOMPAT FBTHRIFTCOMPAT1 FBTHRIFTCOMPAT2
3: Renamed renamed;
4: Replaced replaced;
5: Removed removed;
Expand All @@ -104,6 +101,7 @@ struct DirectoryRenamed {
1: eden.PathString from;
2: eden.PathString to;
}

struct CommitTransition {
1: eden.ThriftRootId from;
2: eden.ThriftRootId to;
Expand Down

0 comments on commit 10c03e8

Please sign in to comment.