Remove ability to reject reentrant ops #20621
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
github-actions
bot
added
area: dds
⯆ @fluid-example/bundle-size-tests: -2.95 KB
Baseline commit: 35aacbd |
vladsud
added a commit
that referenced
this pull request
Apr 12, 2024
…Loader in versioned tests (#20635) An extraction from #20621 Reason for changes: These tests explode when I change interface between DDS & DataStore These tests are not written correctly. They mix arbitrary version of DDS with the latest version of FluidDataStoreRuntime (through TestFluidObjectFactory). We never supported such mixes - version of DDS has to match version or data store! Using matching TestFluidObjectFactory is easy, but then we start running into somewhat similar problem of mixing FluidDataStoreRuntime & ContainerRuntime. We support only N/N-1 mixes here, and nothing more. So we have to use the version provided by test framework, thus you see some dance above CodeLoader to get right factory. And while we are at it, it would be great for the test to use Loader version provided by test framework, not latest version. This breaks for old versions as the way test is written it works only for latest structure of the document. Previously test framework was running a ton of combos, but most of them were actually the same, as even when it was supplied different versions of loader, we were using only latest version of loader. So, the right fix here - use loader version provided by framework but skip all the tests that use old version of the loader, thus getting same result, but with fewer iterations (and faster tests). As part of fixing it, use proper back-compat compatible ways to get to entry points at loader & runtime layers.
…to RemoveEnableOpReentryCheck
vladsud
added a commit
that referenced
this pull request
Apr 16, 2024
…re, runtime, Loader in versioned tests (#20650) An extraction from #20621 Reason for changes These tests explode when I change interface between DDS & DataStore Description These tests are not written correctly. They mix arbitrary version of DDS with the latest version of FluidDataStoreRuntime (through TestFluidObjectFactory). We never supported such mixes - version of DDS has to match version or data store! Using matching TestFluidObjectFactory is easy, but then we start running into somewhat similar problem of mixing FluidDataStoreRuntime & ContainerRuntime. We support only N/N-1 mixes here, and nothing more. So we have to use the version provided by test framework, thus you see some dance above CodeLoader to get right factory. And while we are at it, it would be great for the test to use Loader version provided by test framework, not latest version. This breaks for old versions as the way test is written it works only for latest structure of the document. Previously test framework was running a ton of combos, but most of them were actually the same, as even when it was supplied different versions of loader, we were using only latest version of loader. So, the right fix here - use loader version provided by framework but skip all the tests that use old version of the loader, thus getting same result, but with fewer iterations (and faster tests). As part of fixing it, use proper back-compat compatible ways to get to entry points at loader & runtime layers.
packages/test/test-end-to-end-tests/src/test/pendingBatchReentry.spec.ts
Outdated
Show resolved
Hide resolved
…to RemoveEnableOpReentryCheck # Conflicts: # packages/test/test-end-to-end-tests/src/test/deRehydrateContainerTests.spec.ts
kian-thompson
approved these changes
Apr 19, 2024
12 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.
Remove failed experiment from repo.
It has been proven that pursuing this direction is not feasible / too expensive and does not provide good programming model.
While it does not close venue for some misuse, we have covered some aspects of it (properly tracking referenceSequence number / rebasing for ops sent via re-entrancy).
See https://dev.azure.com/fluidframework/internal/_workitems/edit/2322#12286846 for some details.
Key changes:
Please note that opReentrancy.spec.ts has plenty of test coverage here, including UTs like "Eventual consistency with op reentry..." that uses SharedString to validate proper rebasing happens when dealing with reentrancy.