[#896] Open the entry containers of a backend inside the write and register them after it - #1002
Open
vharseko wants to merge 1 commit into
Open
Conversation
…ide the write and register them after it RootContainer.open opens and registers the entry container of every base DN in a single storage.write, and Storage.write replays its operation after a transaction conflict. No rollback reaches the registry, so a replay found every base DN the attempt it replaced had reached already registered and failed with ERR_ENTRY_CONTAINER_ALREADY_REGISTERED: the backend did not open at all, on a message which said nothing about the conflict that caused the replay. Every attempt also left the configuration listeners of its entry containers behind. The containers are now opened inside the write and registered once it has committed, and every attempt begins by giving up what the previous one opened, as BackendImpl.changeBaseDNTrees does for the base DN change of OpenIdentityPlatform#907. An open which fails closes what it opened, for the same reason. Reached on persistit through a plain rollback, and on the jdbc backend through the conflict and dropped connection replays of an existing backend, whose open issues no committing statement; je and cassandra replay no write at all.
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.
Fixes #896
RootContainer.openopens and registers the entry container of every base DN inside a singlestorage.write, andStorage.writereplays its operation after a transaction conflict - "In case ofa write operation rollback, implementations must ensure the write operation is retried until it
succeeds", whose
WriteOperationis required to be idempotent. No rollback reaches the registry, soa replay found every base DN the attempt it replaced had reached already registered and failed with
ERR_ENTRY_CONTAINER_ALREADY_REGISTERED: the backend did not open at all, and what the operator sawsaid nothing about the conflict that caused the replay. Every attempt also left the configuration
listeners of its entry containers - five per container, plus one per attribute index and per VLV
index - registered on the backend configuration.
The change
The shape is the one #907 / #914 already established for the sibling path: work a rollback undoes
goes inside the write, work no rollback reaches goes after the commit.
RootContainer.opencollects the containers the write opens and registers them once the write hascommitted. Registering after the commit cannot fail here - the base DNs come from a
SortedSetandthe map of a root container being opened is empty until then - so
ERR_ENTRY_CONTAINER_ALREADY_REGISTEREDbecomes unreachable from this path rather than merelyunlikely.
BackendImpl.changeBaseDNTreesdoes: its trees are gone with the rollback, and its containers still hold their listeners.
wrong error for a silent leak.
openAndRegisterEntryContainersbecomesopenEntryContainers, which no longer registers anything.Nothing inside the write reads the registry, which is what makes this available: an
EntryContainerasks its root container for
getCompressedSchema()and nothing else, and every caller ofgetEntryContainer/getBaseDNsruns afteropen()has returned.Which engines reached it
RollbackException, bounded since PDBStorage.write() replays a rolled-back transaction without any bound, and configuration changes hold an entry container's exclusive lock across it #921 / [#921] Bound the transaction replay of PDBStorage.write() #937 but replayed all the same.openTreeis guarded (isExistsTable,isExistsIndex, the catalog row committed on a connectionof its own), so the attempt commits nothing,
partlyCommittedstays false andreplayReasonreplays both a class 40 conflict and a dropped connection. The storage layer keeps that write
replayable deliberately - see the comments at
openTreeandenrolInCatalogabout "a deadlock atthe twentieth tree" - so the Java side was the only thing breaking the contract.
Tests
ReplayedOpenTest, built on the harnessReplayedConfigChangeTestintroduced in #914 - a storagewhich raises PersistIt's own
RollbackExceptionfrom inside the operation, so the replay is driven byPDBStorage.write's retry loop rather than by a second call to it:attempt being replaced - the case the report is written from;
commit(), which replays an operation that ran to completion, and whichalso asserts that the replay gave up both containers of the previous attempt;
All three fail on master with
ERR_ENTRY_CONTAINER_ALREADY_REGISTEREDwrapped inERR_OPEN_ENV_FAIL,or with no listener ever deregistered.
Run green with the change:
ReplayedOpenTest(3),ReplayedConfigChangeTest(12),PersistentCompressedSchemaTest(9),OnDiskMergeImporterTest(29),PDBTestCase(35),EncryptedPDBTestCase(35),PDBStorageTest(10),JETestCase(35),EncryptedJETestCase(35). Thejdbc engine suites need Docker and were not run locally; the change carries no jdbc specific code.
Not in scope
The listeners an
EntryContainerabandons when its ownopen()throws, and the storage a failedRootContainer.openleaves open, are #993.