Skip to content

feat(firestore): Add support for 16MB documents#13478

Draft
dlarocque wants to merge 1 commit into
mainfrom
dl/large-doc
Draft

feat(firestore): Add support for 16MB documents#13478
dlarocque wants to merge 1 commit into
mainfrom
dl/large-doc

Conversation

@dlarocque

Copy link
Copy Markdown
Contributor

Supports 16MB documents out of the box, so only added tests. The tests only run if the RUN_LARGE_DOCUMENT_TESTS = true is set. These tests rely on a seeded database.

Here's the command used for testing

FIRESTORE_RUN_LARGE_DOC_TESTS=true \
    FIRESTORE_TARGET_BACKEND=NIGHTLY \
    FIRESTORE_EDITION=ENTERPRISE \
    FIRESTORE_DATABASE_ID=<db-name>\
    GOOGLE_CLOUD_PROJECT=<project-id> \
    GCLOUD_PROJECT=<project-id> \
    mvn test -Dtest=ITLargeDocumentTest -Dsurefire.failIfNoSpecifiedTests=false

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new integration test class, ITLargeDocumentTest, to verify Firestore's handling of large documents, covering scenarios such as reading, querying, watching, transactions, pagination, and oversized payload rejection. The feedback suggests optimizing the generation of the 16MB payload in the oversized payload test by replacing the inefficient StringBuilder loop with a pre-allocated char array and Arrays.fill() to reduce CPU and memory overhead.

Comment on lines +143 to +148
int targetBytes = 16 * 1024 * 1024 + 102400;
StringBuilder builder = new StringBuilder(targetBytes);
for (int i = 0; i < targetBytes; i++) {
builder.append('a');
}
String largePayload = builder.toString();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Creating a 16MB string using a StringBuilder in a loop with over 16 million iterations is highly inefficient and can cause significant CPU overhead and garbage collection pressure during test execution.

Instead, you can pre-allocate a char array, fill it using Arrays.fill(), and construct the String directly. This is much faster and more memory-efficient.

    int targetBytes = 16 * 1024 * 1024 + 102400;
    char[] chars = new char[targetBytes];
    Arrays.fill(chars, 'a');
    String largePayload = new String(chars);

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.

1 participant