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

Update USN journal entries for projected folders #1664

Merged
merged 8 commits into from
May 27, 2020

Conversation

derrickstolee
Copy link
Contributor

@derrickstolee derrickstolee commented May 22, 2020

This replaces #1648.

BuildXL was having issues detecting that a folder had changes when the projection changed because VFSForGit was simply updating its in memory projection and not doing anything with the file system that would trigger and update to the USN. It would hand back the new items in the directory when ProjFS would ask for the items for enumeration the next time an enumeration happened.

This change is to make the folder get a new USN when VFSForGit determines that a folder has changed. This is done by keeping a SHA1 hash of all the names of the folder entries that are in the projection. When the projection changes, calculate the new SHA for the folder and compare with what it had before. If it changed, then write and delete a file in the folder to trigger the USN update.

Note: We don't need any upgrade step because we are already storing the SHA for the folder in the placeholder database but it is currently null for all folders so the first time there is a projection change it will cause all the folders to get the calculated SHA for the folder and update the placeholder database.

Tasks:

  • Update folder SHA-1 calculation to depend only on child names, not contents.
  • Get installer to BuildXL folks for validation.
  • Place this behavior behind a config option.
  • Document the config option. (Delaying to Documentation: Add in-repo docs #1665)
  • Update functional test to run in CI and PR validation (behind config). (The test is updated to work behind the config, but the fsutil usn readdata <path> command isn't working in CI. Tested locally.)

kewillford and others added 6 commits May 22, 2020 09:39
To update the USN journal entries for a projected folder, we
previously computed the new hash based on the contained files and their
contents.

Let's also include the file and folder names.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
@derrickstolee derrickstolee force-pushed the usn-folder-update branch 2 times, most recently from 644e051 to 00b7983 Compare May 26, 2020 12:40
@derrickstolee derrickstolee changed the title Usn folder update Update USN journal entries for projected folders May 26, 2020
@derrickstolee derrickstolee marked this pull request as ready for review May 26, 2020 14:09
Copy link
Member

@kewillford kewillford left a comment

Choose a reason for hiding this comment

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

This looks pretty good. Just a couple of clean up comments. I'm going to be out the rest of this week but will approve knowing you will consider the suggested changes.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Add new "usn.updateDirectories" boolean config option. This is set in
the system-wide config, and requires remounting for the config to take
effect.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
@derrickstolee
Copy link
Contributor Author

I got final testing from the Office folks. Merging now.

@derrickstolee derrickstolee merged commit 90deaef into microsoft:master May 27, 2020
derrickstolee added a commit that referenced this pull request Jun 2, 2020
Includes the following pull requests:

* #1663: Update Git to v2.27.0
* #1669: Docs: more troubleshooting and FAQs
* #1668: ProductUPgradeTimer: remove unacceptable Environment.Exit()
* #1664: Update USN journal entries for projected folders
* #1666: Docs: start basic template
* #1658: ProjFSFilter: Be more robust to missing PowerShell
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.

2 participants