[Drop, 97 changes] Sync through 2022/11/18 17:00 - Pre-unix-gcr1#59
Open
MsftBrettShirley wants to merge 102 commits into
Open
[Drop, 97 changes] Sync through 2022/11/18 17:00 - Pre-unix-gcr1#59MsftBrettShirley wants to merge 102 commits into
MsftBrettShirley wants to merge 102 commits into
Conversation
added 30 commits
September 27, 2025 09:56
Compile with C++17
Note: Invoking with:
<PreprocessorDefinitions>_HAS_STD_BYTE=0;_HAS_AUTO_PTR_ETC=1</PreprocessorDefinitions>
_HAS_STD_BYTE is to fix that 'byte' is defined in rpc headers as well as in std.
_HAS_AUTO_PTR_ETC=1 turns back on a few language elements that were removed from C++17
random_shuffle()
mem_fun()
auto_ptr()
Removing our 1 use of dynamic exceptions, a feature removed from C++17.
[Substrate:b9aa2bf313b1fdfc4132e86d29bf7c8b5bab8f86]
Some compilers have problems with our C_ASSERT macro. This check-in creates a GCR style tool to bulk replace, but does not yet bulk replace. It also tweaks some .pl files that won't be caught by the bulk replace tool. The next checkin will be the bulk replace. There will be one more after that that does a few things by hand-editing (like removing our macro definition). [Substrate:29cb5ff81f2f1a4b02eb95b516f49f17ab465940]
Added the possibility to subsample a complete trace in resmgrreplay according to the method presented in Waldspurger, C. A., Park, N., Garthwaite, A., & Ahmad, I. (2015). Efficient MRC construction with SHARDS. In FAST 15. Subsampling the cache trace according to this method should enable to simulate the impact of different cache sizes or checkpoint depth from a much smaller trace without lose much accuracy. This change in resmgrreplay helps to measure how much accuracy can be lost due to subsampling. [Substrate:8a9ee7ee7971f6ad705449d3e8defef80cd1dfb4]
…tion. "If there aren't any good wheels around, just invent your own." - Unknown author. We've had a C implementation of an event log reader since the days when we would mostly write tests in native code, so this change implements an interop layer on top of that implementation and replaces the System.Diagnostics.EventLog uses throughout our test code. That class has a few bugs that have called for ugly workarounds, which have now been removed. [Substrate:b8a242c84f7d5ce27da55d5bc80cda1b8b42693d]
…ncile page if it is within required range [Substrate:50d6edd67b3d62b8d5d2e4f5ce69c31cd4b62a42]
…sion store cleanup. Also, trigger version store cleanup task when read-only transaction check for max transaction size [Substrate:3afad5cd480190a23d9d35c45c87488f103f46dc]
…hold 1. We will truncate RBS if we use more than 300GB of disk space. 2. We will also start loose truncation at a higher threshold of 200GB i.e., if we have lesser than 200GB of disk space left we will start truncating the snapshots. [Substrate:906a927437029057120afd6c152b744c39db0544]
- Implemented a more aggressive retry strategy. - Added failed conflict resolution to telemetry. [Substrate:eadb3be0373aee4441571e003f6b3f524398be95]
Sync Windows/ESENT changes through commit 4a0e169cb9793. [OS:4a0e169cb9793] [Substrate:241509805d387723bee5196aab52decaf41a2a69]
Added the possibility to trace a subset of pages in cache according to the method presented in Waldspurger, C. A., Park, N., Garthwaite, A., & Ahmad, I. (2015). Efficient MRC construction with SHARDS. In FAST 15. This change makes possible to collect enough data to be able to simulate and study cache performance (for example with difference parameters such as the cache size) with a much lower overhead since only a few pages are traced. The subsampling is activated only if the keyword 'BFRESMGR' is set when collecting the trace, and the ratio of sampled pages can be set with the parameter 'JET_paramFlight_CacheTraceSamplingRatio'. [Substrate:217dee6e6b8e0d3732d4b7a1a661f01928cc6353]
…ksum format flag. If old checksum format is detected and the checksums don't match, then rechecksum with new format and compare again. If the ECC checksum indicates a correctable error, then it must be the new checksum and the bit can be corrected. [Substrate:ae6b019edfe10005d3d3aeb9f3758cd0975ce848]
Modify jethdr.W to not use base types like "long" or "wchar_t", but rather used wrapped base types like JET_UINT32 and JET_WCHAR and JET_PSTR/JET_PWSTR where appropriate. This abstraction will be used when compiling Linux and those two types are different there. Also, fixed a bunch of indention/formatting. For example, normalized placement of the "*" character when defining a pointer param, some use of single-line comments, etc. [Substrate:34324ee3abf5f10ba83f11fce5ec21758cd62263]
…string in CPAGE::ErrCheckPage() [Substrate:298df9ad4f45dfaf930c821c680a9901d2d3e8a9]
This increases the size of the presence filter from 8 bits to 10 bits. The code is written so that it is easy to change it again in the future as long as each bucket is byte aligned. [Substrate:294faf2955f563d2a84f6b3b8f8433344b5358a8]
…eRead In prod we are still spending a huge amount of time in RequestFinalizeRead accessing slabs. This is true even though we don't cache reads. We also don't seem to be hitting the cache very much because ErrVerifyCluster is a very small percentage of the cost. This change causes each CRequest to remember if it had a cache hit or miss. We can then use the cache hit flag to rule out accesses to the cache. This could allow us to avoid accessing the slab. This is more certain than using the presence filter. If this does work then we may need to look more into the effectiveness of the presence filter. [Substrate:7464b6e75b808cf65e14ae076908f3474fd8ec5f]
…DB, not fail with JET_errOutOfDatabaseSpace FShrinkIsRunning() means that the engine is currently running code under Shrink. FShrinkIsActive() means that, in addition to Shrink being running, it's also still actively trying to move pages to lower offsets to truncate the file. The transition from FShrinkIsActive() true to false (while still keeping FShrinkIsRunning() true) happens when we're faced with a scenario where not growing the database could lead to leaked space, so we give up shrinking at that point. Recently, we saw cases where Shrink was being disabled and tried again to grow the DB on the same stack. That caused Shrink to fail with JET_errOutOfDatabaseSpace, instead of succeeding and bailing out more cleanly with sdrNoLowAvailSpace . This bug only affects MCDBs, which have a max DB size set, and doesn't have any practical impact, other than just optics listing failed Shrink runs, when they should have been reported as benign cases where we don't have space below the target to allocate space anymore. The fix here is to return the benign errSPNoSpaceBelowShrinkTarget error even if Shrink is inactive, but still running. [Substrate:cc0cf8193e3da0101994d130f2c8c62bd4a17a92]
…ing file does not support FileIdInfo During testing, it was discovered that accessing files in certain cases (e.g. \\tsclient\c\foo.txt) failed because GetFileInformationByHandleEx with FileIdInfo was failing with ERROR_INVALID_PARAMETER when the file id was being queried for the file while it was being opened. This change traps this case and causes an invalid file id to be returned. This case was already established to be valid for a file that cannot ever participate in EBC caching. [Substrate:b8ce56bd1da9fef79a88d092da7520abce640908]
The cuckoo filter reference implementation contained a standby list to hold any item that couldn't fit in the main table. I implemented this because this is the kind of thing that isn't added without a good reason. The problem is that while we faithfully add/remove items from the standby list, we forget to consult it on the set membership check! So we can return a false negative for an item in the standby list which is not allowed. This fix consults the standby list during the set membership check. [Substrate:dbc2e5d499d2ff297f07ce3ea28b59cd6c202cb1]
[Substrate:40691275fbb2f5a0103f73cf7d646019e7e9991f]
…ess or dehydrate flags [Substrate:77921ebeb87905b214b643dc767869412d76f568]
…applied in a loop using dbtime to uniquely identify the record [Substrate:ab598789a5d286a2e3b6e9419a5e346fcd79a3b0]
…have PageFDPDelete flag set For ExtentFreed records for non-revertable deletes, we will validate the root page has FDP delete flag set if db was reverted i.e., current lgpos redo is before lgposcommitbeforerevert and we are not within the required range. [Substrate:025d113a262b36ed2aa2e3fee62f4fd59f3745bd]
…e filter during eviction When we load a slab for the first time, we enumerate the entries and load the presence filter appropriately. On any update, we analyze the update and make a relative change to the presence filter. Because of how this works, we can only consider the contents of each updated slot to make the decision to add or remove entries from the presence filter. It turned out that tracking any "clean" block or the latest version of a "dirty" block was practical. When a block is evicted, we will remove it from the PF if it was a "clean" block or the current "dirty" block. When a block is cached, we will add it to the PF if it was a "clean" block or the first "dirty" of a block. This may result in two PF entries for the same block if we cached the "clean" page on a read miss and then dirtied it (until that obsolete image is evicted). This was a compromise because there is no state to determine that the first "dirty" of the block overrides a "clean" block in our current scheme. The bug was that my code to find the "clean" block was slightly incorrect. It recognized any clean block including one that was previously dirty, had been written back, and was now being evicted. I was testing !FDirty when I should have been testing !FDirty && !FEverDirty. This change also includes: - some cleanup to ErrLoad to clarify what it is doing - more fixes to the presence filter related to the standby list - remove wasn't updating the standby list - retiring a standby item had bad flow control - moved the call to Update from ErrUpdateSlabs to ErrScheduleSlabForWriteBack to clarify/guarantee it is always called for any slab update and document that it is only done at do time not redo time Other changes: - synctool.bat updated to find Product Studio binary that was moved [Substrate:2488950c19bed3c44f9cdbc0cf8ec27319035f66]
…ad of a bool [Substrate:e7ff29f83a84fff2980e250238cff9de28672fb9]
…ts >= 16TB When inspecting the code, I noticed that we have a potential corruption issue related to integer truncation. The HashedLRUKCache caches files using 4kb blocks. It only reserves 32 bits to store the block number. It also reserves 2^32-1 as an invalid block number. If the accessed offset of a file / 2^12 >= 2^32-1 (i.e. about 16TB) then the cache truncates the block number and Bad Things will happen. This change addresses this by preventing caching of these offsets. It doesn't seem likely at this time that we will have dbs in prod that are this large so this is fine for now. [Substrate:f8b8ec23f64da9e64e78651ac496ccf2c7f36366]
… a file to the cache The storage may legitimately not be present. We already ignored this error This avoids the noise for SSD Only Databases in production. [Substrate:a57badca49385ee3b4eed14a3378a3c58185b73b]
When the EBC master enable was switched off, we tried to run recovery against a cached EDB file. This resulted in a recovery failure but with the weird error JET_errPageSizeMismatch. We should have failed with JET_errFileInvalidType instead. This change adds an optional type check to ErrUtilReadShadowedHeader etc. The caller must indicate which type of file they are attempting to access. If the file may be of any type then JET_filetypeUnknown may be passed. We will also allow JET_filetypeTempDatabase if JET_filetypeDatabase was indicated (i.e. accept the more specialized type). This change also cleaned up the cut&paste code duplication in ErrUtilReadSpecificShadowedHeader. [Substrate:af3a3ef50f46ad12f310a0aeaad5e471fdc95d2f]
Tiny tweak so that the code is the same for esent and ese versions of eseutil, just a different data. Makes it simpler to remove strsafe.h. [Substrate:1c5be300a2241b54dcb1c15700cb09a318844641]
…ontexts BF has code to detect hung IOs. It does this by capturing the pvIOContext provided by IFileAPI::PfnIOHandoff and later uses this to call IFileAPI::DtickIOElapsed. The problem with this code is that it can use the pvIOContext after it has been released by the IFileAPI implementation. Specifically, this happens because BF captures the pvIOContext in calls surrounding sync I/O calls such that there is a timing window where the pvIOContext has been released but it hasn't been removed from the associated BF. For COSFile, this is a minor annoyance because pvIOContext is an IOREQ and these are never released. Worst case, BF will check the wrong request. If it does, chances are excellent it won't be hung because obviously it was just reused. For the ESE Block Cache, this is a major annoyance. EBC's file system filter had to build an expensive scheme to validate these requests and return a safe answer if the CIOComplete had been released already. We had to pay this penalty even for files that were not attached to a cache. Even a probabilistic scheme like using a TPool<CIOComplete> with a min lifetime of 10s was not enough to avoid this hazard. This change addresses the root cause of this problem by eliminating the concept of a pvIOContext for sync I/O. A NULL will always be returned by IFileAPI::PfnIOHandoff for a sync I/O. BF has been changed to compensate for this by complicating the usage of BF::pvIOContext. For an async I/O it stores the pvIOContext from IFileAPI::PfnIOHandoff. For a sync I/O, it stores a TICK. These are distinguished by the least significant bit, which is set for a TICK and clear for a pvIOContext. BF uses the TICK to directly track the elapsed time of sync I/Os. EBC's expensive compensation scheme for this hazard was also removed and replaced with a pooled allocation for CIOComplete. [Substrate:bb29f616fd6a88422f6f2e74b690ab5b8d039b2f]
added 24 commits
May 18, 2026 11:03
On passives, RCEs can be allocated out of order because of deferred RCEs which are only allocated later if needed. Cleaning them up in that allocation order can result in RCE chains during the cleanup which are illegal. So, need to clean them up in logical order. Also added some asserts to make sure deferred RCEs can only be used while in the initial required range. [Substrate:c84b5929fb7e2e358ed7d8a2fbc84b9dcb4ee810]
The largest impact of this change is that we change how CRequests are processed by the AsyncIOWorker so that it handles IO handshaking via just the master CRequest per IO rather than checking all CRequests in the IO. This results in the AsyncIOWorker being cued far less often (once per IO rather than once per CRequest). For similar reasons, we also call IFileAPI::ErrIOIssue less often. Other changes: - CRequest now directly owns the pointer to the associated TLS (if async) and manages its lifecycle. this also improves debuggability - CRequest has more debug tracking state for request and issue of IO - CRequest allocation is now pooled - CRequest::WaitForIORangeLock now directly cues the AsyncIOWorker via the TLS rather than using an abstracted delegate - TPool cleanup was becoming unmanageable so a static self registration scheme was added to enable cleanup of all TPool instances with a single call [Substrate:afb3521f0b279885728c7c49ae68cffd687c6ba7]
…ped earlier RCE in chain [Substrate:87ebc9d674f60a7074df3059c09e6c8d3c78f58d]
This code currently uses a complex scheme with a mutex to sync access to the ESE.TXT trace file. This change opens the file with FILE_APPEND_DATA. This makes any WriteFile call an append. We can then simply just call WriteFile w/o any synchronization to append data. [Substrate:28117c8873a8d6c9d02aafae570a146aa5205f91]
1. Separate out FCB creation from FUCB creation. Move FCB creation to the FILE layer, where catalog properties can be accessed. FCB initialization is separated into 2 parts. Required init, and optional init. 2. Provide a mechanism to refcount FCBs without opening FUCBs on them. The FILE layer guarantees that any pfcbs it passes around are properly refcount managed. This is change 1/2. Some CR feedback will come later in change 2/2. [Substrate:8e3b5f6f07f2590ae8fd539df59a088d8b66931e]
[Substrate:420b32c1b9212c12f789ac2943a227dbe9eaafe6]
[Substrate:3755b45d2ed0e28602144622fef57629774cf110]
They are two separate but similar issues: 1. Verstore fix is for a case where we have a flag delete RCE after a table delete RCE. This can happen because of verstore cleanup of delta RCEs doing the finalize action on a delta column. The change is just copying the assert at line 5880 for the pfcbLV case. It can also happen on a regular table if it has a delta column with JET_bitColumnDeleteOnZero. 2. The DIR fix is a case where we are opening a proxy cursor during concurrent index create, and the assert isn't happy because the proxy PIB didn't open the database. The proxy PIB is a system PIB used in the execution of a finalize task to do a flag delete operation. We don't call ErrDBOpenDatabase() for any DBTASKs. So it is just relaxing this assert for this specific case. [Substrate:26eee8f68957ae38a85cbeea7794441147f7a26d]
…terIFilePerfAPI During local stress, we hit a case like this: - The IO thread has started several async requests, each of which has a read count on the RWL. - The other thread has opened the file (already open) and is registering an IFilePerfAPI but blocks because of pending IO. - The IO thread starts another request which blocks because the other thread is waiting its turn. - Deadlock. This is similar to an earlier problem we fixed where the thread calling RegisterIFilePerfAPI deadlocked because it had previously requested IO. The fix is for any thread interacting with this registration RWL to detect if they will block and, if so, call ErrIOIssue first to prevent the deadlock. [Substrate:6bff8a98a2f947c8a82cf91f748339f5e8862112]
…mp in .IRS.RAW [Substrate:8a6c0fcb2ed21d62a09b4cfcbbb84d7ed2c51a47]
Recently we hit an assert where the FCB hash table was expanding and failed an assert that an entry in the table matched its key. This happened during a store.worker schema upgrader. This revealed that we have an ancient issue where if someone is modifying a template table via JET_bitPermitDDL then we should be protecting accesses to the table schema with EnterDML. This is currently skipped for tables with fixed schema like template tables. The fix is to EnterDML for template tables if their schema is not truly fixed. We detect this via the existing FTemplateStatic(). Note that this results in EnterDML being required for updates to template tables which is supported but very rarely used. This change also significantly improves the UnpublishedJetConvertTests. [Substrate:a7ac027d4d3439608c08509b7ddfab8faa817d92]
In prod, we are spending a lot of time manipulating journal slabs during the evict process. We are using them to allocate clusters to replace clusters that hold evicted data to support our recovery scheme. This process is effectively a FIFO allocation scheme and so it should be cheap. We actually end up using a lot of CPU doing this because we scan the entire slab twice to find the next cluster. In prod, we end up calling ICachedBlockSlab::ErrGetSlotForWrite 16x per database page. This change optimizes this case as follows: - each slab now tracks the count of invalid slots and the lowest invalid slot number - each journal slab now uses the cached lowest invalid slot to directly find the next cluster - ErrEvictOrInvalidateSlot uses the count of invalid slots to determine if it needs to flush to reuse the clusters in a journal slab - ErrEvictOrInvalidateSlot no longer calls ICachedBlockSlab::ErrGetSlotForWrite twice for every operation Other changes: - We now use a CJournalSlabWrapper to manage the ref count on the journal slabs, cleaning up some code. - The alloc of CCachedBlockSlab and CCachedBlockSlabReference and CJournalSlabWrapper are pooled - CHashedLRUKCacheThreadLocalStorage::FOpenSlab and FAnyOpenSlab were changed to use the ibSlab as the key to allow wrappers of ICachedBlockSlab like CJournalSlabWrapper to be used [Substrate:678929bcce24d55713a1ebb161e23f41c2aabc8a]
…ways nullify RCEs in order [Substrate:e5e3c04eab25511212fa09aa1836d6a2e5011842]
[Substrate:0d40336f41a9bb00df2c23df38982f23242d5987]
…an ESE full test pass. The reverted PR age: 25.63 hours old. [Substrate:f67bf12757b9af6ecb1901f0efdc901242515ba2]
Deactivated the tracing of EBC events that were using the same ETW events as the BFResMgr. This is required in order to use the sub sampling of the cache traces: EBC produces significantly more events than a subsampled BfResMgr, which produces large trace files, defeating the point of subsampling. Subsampling could be added to EBC tracing later. [Substrate:55439d4abe0f32d6b53c2b1dd0930e6f7b0b559c]
When GCR2 is run and we collapse all "long"s to "int"s, we end up needing some casts to make the OS happy. Making a small number of those casts here. They're NO-OPs in the current code. Also, first example of the #define foolery we need to make DWORD work. DWORD in Windows is based on long. If I was being harsh, I'd remove all usages of DWORD from our code. But, DWORD is too deep in our psyches to do that. So, GCR2 will redefine DWORD to be based on "int" and then we have to play a #define game to be able to include Windows headers without type redefinition errors. Far in the future I see some reorganization of the #include order so that windows specific headers are included in far fewer places. [Substrate:4463f3c49fff90ba1ded53f1ba56886ea5cb6ad8]
Currently, EBC is written such that if a sync evict is needed to cache something then that evict is processed and journaled separately. The subsequent update to cache something is its own journal entry. This is obviously not optimal because we end up processing and journaling the same slab twice per update. This change reorganizes the code slightly so that these updates are combined. Now, instead of seeing one journal entry evicting an entry and another one caching an entry in its place, this will be combined into one journal entry where it looks like we just replace one entry with another. To aid in analysis and debugging, we will also emit the before image of any evicted slot in the combined journal entry. This slot will be applied to the target slab but will be immediately overwritten with the subsequently added after image. This impacts several key code paths that analyzed the changes in a slab on update: - CUpdateSlabVisitor verifies that the cluster is replaced on any evict/invalidate as this is required to support our recovery scheme - CUpdateSlabVisitor collects slots that are involved in the update - CCachedBlockPresenceFilter::FUpdateSlot updates the presence filter based on evict/cache as required to correctly identify items that may reside in the cache - LogCacheUpdateJournalEntry => TraceCacheUpdateJournalEntry emits a trace on every cache update at do or redo time - ErrDumpJournalEntry dumps each entry in the journal These code paths were updated to use more nuanced checks to still allow the detection of an evict/cache even if it happened to the same slot in one atomic operation. Some other fixes were made: - CCachedBlock::Updno() was exposed to be public because it is used by the new logic above to detect an evict/cache in the same slot using the tuple VolumeId, FileId, FileSerial, CachedBlockNumber, and UpdateNumber to uniquely identify a version of a cached file block - We defined an updnoFirst and explicitly prevent it from being used because there is logic in CCachedBlockPresenceFilter that relies on this value uniquely identifying the first update of an item [Substrate:fc1a83b964e76297b144b8b011919a1bed0e285c]
…ep limit at end of recovery This means that the first operation after JetInit will not fail with CheckpointDepthTooDeep again. We could potentially optimize this by just running an iteration of checkpoint maintenance and update but that is not easily to run synchronously. [Substrate:e406536518b1d85abcb05d172ba6a58801e4e617]
This change contains: 1. New data structures for the BBT buffer and nodes stored in it. 2. Unit tests. 3. Debug extension. 3. Fix CPAGE::RevertDbtime() to stop changing arbitrary flags. It only allows the revert of fPageScrubbed and verifies the revert against cached state. The new data structure isn't hooked up to ESE yet. Only unit tests will exercise the new code. [Substrate:506e0ba2405fb79a7f1753955b2d8d207c6b3824]
…nd Clear() whenever possible. This change adds three new methods to CArray: - ERR ErrAppendEntry( const CEntry& entry ); - bool FRemoveLastEntry(); - void Clear(); It also replaces common patterns throughout the code with these new methods. [Substrate:76ec4ee374b60a66a56f88b94496417079ce3721]
Note: I haven't yet been able to debug this from the dump, but this is almost assuredly the issue. [Substrate:6da1bbb9fb5b2d492770026db9b858fa1a59e38f]
…CE as the preceding one can be an uncommitted Delta RCE [Substrate:a12c014d42b9048e5e443d5d42aa004b78fb7911]
…en moving data. **Summary** This change implements the ability to move multiple pages which are contiguous within an extent to a destination extent which is tailored to accommodate those pages without creating fragmentation, while also keep the original contiguity aspect of that set of pages. This is a pre-requisite to Hybrid Shrink because we will be operating potentially on large portions of live databases, so we do not want to break existing contiguity, which would be particularly detrimental to HDD-hosted databases. The code will already start running as part of attach-time Shrink, when we come across leaf, non-space pages. Currently, we move those pages one by one without any awareness of contiguity. **Details** - Added new flight param JET_paramFlight_ContiguousExtentMoveShrinkEnabled to switch the feature on/off. By default, it'll be **off** in retail ESE and **on** in debug ESE. However, clients above ESE will start with it always **on** during the test pass, as well as in PDT and NAMPR00DG006. - Added new flight JET_paramFlight_HierarchicalSpaceAllocFlagsEnabled. In the process of developing this change, I found a bug where some space request options were not being passed along when space was requested to the parent object of the current tree. This parameter turns that fix on/off. It'll be checked-in with it always **on**, but I've decided to add the flight to turn it off in case it is needed. - Changed wrnBTShallowTree to errBTShallowTree. The motivating factor was that the warning was being returned in cases where the requested operation was **not** actually performed, so it was semantically wrong, in addition to forcing code to always check specifically for that warning. - Implemented ErrBTContiguousExtentMove, which is the core of this change. - Updated the current attach-time Shrink code to use the new feature. - Renamed BOOKMARK_COPY to BOOKMARK_BUFFER and added some functionality. - Added new test case to cover new functionality. - Fixed a test case that broke due to the stricter requirement to move pages (contiguous destination extent). [Substrate:38dda6171b4f83a7419453edef8c6c8e7fe46e38]
anilruia
approved these changes
May 22, 2026
XiaoshiSha
approved these changes
May 22, 2026
This file contains hidden or 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
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.
Microsoft Reviewers: Open in CodeFlow