Feat - Caching layer to database#4
Merged
eldadfux merged 18 commits intoutopia-php:v0from Apr 29, 2021
Merged
Conversation
Contributor
Author
|
I can't seem to decode Documents from cache correctly - got any advice? Time: 00:00.301, Memory: 6.00 MB
There were 3 errors:
1) Utopia\Tests\Adapter\MariaDBTest::testCreateDeleteAttribute
Exception: Must define $id attribute
/usr/src/code/src/Database/Database.php:548
/usr/src/code/src/Database/Database.php:291
/usr/src/code/tests/Database/Base.php:51
2) Utopia\Tests\Adapter\MariaDBTest::testCreateDeleteIndex
Exception: Must define $id attribute
/usr/src/code/src/Database/Database.php:548
/usr/src/code/src/Database/Database.php:291
/usr/src/code/tests/Database/Base.php:99
3) Utopia\Tests\Adapter\MariaDBTest::testCreateDocument
Exception: Must define $id attribute
/usr/src/code/src/Database/Database.php:548
/usr/src/code/src/Database/Database.php:291
/usr/src/code/tests/Database/Base.php:127
--
There was 1 failure:
1) Utopia\Tests\Adapter\MariaDBTest::testCreateDeleteCollection
Failed asserting that true matches expected false.
/usr/src/code/tests/Database/Base.php:42 |
Contributor
Author
|
Contributor
Author
|
In 66c9f70, I discovered the following behavior, which has been causing our issues: $document = $this->adapter->getDocument($collection->getId(), $id);
$document !== new Document($document->getArrayCopy())https://gist.github.com/kodumbeats/d910b12983540bdb125219b7791f27fb |
Contributor
Author
|
As of 53d521c: |
eldadfux
reviewed
Apr 29, 2021
4 tasks
premtsd-code
added a commit
that referenced
this pull request
Apr 13, 2026
#1 Drop $enable flag on skipDuplicates() scope guard The $enable param made every non-skipDuplicates createDocuments call pay for a closure allocation + extra function call. Branch at the call site instead so the cost only applies when the flag is actually set. - Adapter::skipDuplicates(callable, bool) → skipDuplicates(callable) - Database::skipDuplicates(callable, bool) → skipDuplicates(callable) - Database::createDocuments, Mirror::createDocuments, Pool::delegate, Pool::withTransaction now branch inline. #2 Drop fetchExistingByIds helper, inline find() The helper's per-tenant grouping defended a hypothetical multi-tenant batching scenario that no caller exercises (relationships are intra- tenant, callers always batch per tenant). Existing patterns in the same file (refetchDocuments, relationship loading) just call find() directly. Match that idiom and drop ~70 lines. #4 Mirror: only capture inserted docs in skipDuplicates mode The captureOnNext accumulator paid the cost (closure + per-doc array push) on every createDocuments call, including the common non-skip path. Branch at the entry of Mirror::createDocuments so the capture only happens when skipDuplicates is set; the non-skip path passes through to source/destination unchanged. #5 Move getInsertKeyword/Suffix/PermissionsSuffix to getters cluster Were sitting next to createDocuments(); moved to the getSupport* cluster around line 1030 where other adapter-capability shims live. Not addressed: - #2 partial: the existing patterns (refetchDocuments etc.) don't handle tenant-per-document multi-tenant batches either, so this is consistent. - #3 (drop the pre-filter): rejected. createDocumentRelationships runs in the encoding loop BEFORE the adapter's INSERT IGNORE no-ops the parent, so dropping the pre-filter would deterministically duplicate child rows on every CSV re-import of a collection with relationships (not a race window — every call). The relationships test verifies this. Reverting would require reintroducing the deferred-relationships scaffolding we just removed, and the adapter still couldn't tell us which parents were actually inserted (SQL INSERT IGNORE has no per-row reporting). Pre-filter stays.
premtsd-code
added a commit
that referenced
this pull request
Apr 14, 2026
Per Jake's #4 review comment, the captureOnNext accumulator could grow unbounded with the full inserted set. Replace it with a bounded buffer that flushes to the destination every $batchSize docs as the source streams them through onNext, plus a tail flush after the source returns. Memory peak in skipDuplicates mode is now O($batchSize) (~1000 docs) regardless of how many docs the source inserts. Same destination batching efficiency — destination still gets multi-doc createDocuments calls, just one per chunk instead of one giant call at the end. Eligibility checks (SOURCE_ONLY_COLLECTIONS, upgrade status) moved upfront so the flush closure can capture the decision and short-circuit cleanly. Non-skip path is unchanged: direct passthrough, no buffer. The flush-on-fill is load-bearing for the memory bound — see the in-method comment.
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
2 participants
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.
This PR allows for the use of a cache when creating a database.