[#708] Fix OutOfMemory during replication initialize with JDBC backend#711
Open
vharseko wants to merge 3 commits into
Open
[#708] Fix OutOfMemory during replication initialize with JDBC backend#711vharseko wants to merge 3 commits into
vharseko wants to merge 3 commits into
Conversation
…IdentityPlatform#708) The JDBC storage cursor fetched the whole tree with a scrollable ResultSet ("select h,k,v ... order by k"): the postgres/mysql drivers materialize such result sets entirely in client memory, so opening a cursor over id2entry during import phase two pulled all entries into the heap and stalled every index writer sharing the connection. Replace the scrollable ResultSet with keyset-paginated batches ("where k>? order by k" + offset/fetch, configurable via org.openidentityplatform.opendj.jdbc.fetchsize), resolve cursor positioning with server-side queries, and create a supporting index on (k) for postgres so ordered batches use an index scan instead of a full sort.
MySQL cannot use a prefix index over "k tinyblob" for the cursor's "where k>? order by k" batches (the plan stays full scan + filesort), so the dialect now uses k varbinary(255) (same 255-byte cap) with primary key(h,k), and openTree creates a full k_<hash> index via a metadata check, since MySQL has no "create index if not exists". Also cover cursor positioning across fetchsize batches with a test on all JDBC dialects.
…e database - test_issue_496 accepts attributeOrValueExists: the shared in-JVM server already has the schema definitions when another backend test class ran first - jdbc TestCase drops stale opendj_% tables before setUp: PgSql and Encrypted map the same tree names to the same tables, and leftovers encrypted with a lost cipher key broke the next run's cleanup
maximthomas
reviewed
Jul 7, 2026
| return "h char(128),k raw(2000),v blob,primary key(h,k)"; | ||
| }else if (((CachedConnection) con).parent.getClass().getName().contains("mysql")) { | ||
| return "h char(128),k tinyblob,v longblob,primary key(h(128),k(255))"; | ||
| return "h char(128),k varbinary(255),v longblob,primary key(h,k)"; |
Contributor
There was a problem hiding this comment.
I guess the migration from tinyblob to varbinary(255) fo MySQL could potentially cause data loss
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 #708
Problem
Initializing a large replica (~40M entries) with the JDBC/PostgreSQL backend fails with
OutOfMemoryErrorduring the index initialization phase, with all indexes stuck at0% completeand PostgreSQL transferring huge amounts of data.Root cause:
Storage.CursorImplopened every tree cursor as a scrollable ResultSet over the whole table (select h,k,v from ... order by k). The postgres driver streams rows only forTYPE_FORWARD_ONLYstatements with a fetch size — scrollable result sets are always materialized entirely in client memory. Import phase two opens such a cursor overid2entry(MostlyOrderedChunk.flip()→ImporterToChunkAdapter.flip()), so the driver tried to buffer all 40M entries in the heap. The giant query also monopolized the single connection shared by all phase-two index writers, which is why every index reported0% complete. The same materialization happened for any cursor in normal operation (e.g. unindexed searches), and no configuration workaround exists (defaultRowFetchSizeis ignored for scrollable result sets).Fix
CursorImplnow iterates with keyset-paginated forward-only batches (where k>? order by k+offset ? rows fetch next ? rows only,limit ?,?for mysql), fully draining and closing each batch statement. Cursor memory is O(batch) instead of O(tree). Batch size is configurable viaorg.openidentityplatform.opendj.jdbc.fetchsize(default 1000).positionToKeyis a point lookup on(h,k),positionToKeyOrNext→where k>=?,positionToLastKey→order by k desc,positionToIndex→offset. Semantics mirror the JE backend cursor (failed positioning leaves the cursor undefined).delete()issues a regulardelete where h=? and k=?instead ofCONCUR_UPDATABLE/deleteRow().openTreecreatescreate index if not exists k_<hash> on <table> (k): the(h,k)primary key cannot serveorder by k, so without it every batch would be a full-table top-N sort. Existing deployments get the index on next backend open.k tinyblobdialect cannot serve the keyset batches at all: the optimizer refuses prefix indexes (k(255)) fororder by kand even for the range predicate — the plan stays full scan + filesort (verified on mysql:9.2). The dialect now usesk varbinary(255)(same 255-byte cap) withprimary key(h,k), andopenTreecreates the fullk_<hash>index through a metadata check, since mysql has nocreate index if not exists. The batch query then runs as an index range scan without filesort, same as postgres. Tables of the previous tinyblob dialect are not migrated and must be re-created.Tests
PgSqlTestCase33/33,MySqlTestCase33/33,EncryptedTestCase(jdbc) 32/32 — includingtestExportLDIFAndImportLDIFandtestRebuildAllIndex/testRebuildDegradedIndex, which exercise the exact import phase-two andID2EntrySourcecursor paths that previously OOMed, plus the newtestCursorCrossesFetchSizeBatches: a cursor withfetchsize=2over 7 keys checks iteration order across batch boundaries,positionToKey/positionToKeyOrNext/positionToIndex/positionToLastKey,delete()through the cursor with continued iteration, and the read-only guard. Verified in postgres and mysql thatk_*indexes are created on all trees and serve the keyset batches with index scans.Bundled test-infra fixes so the classes also pass in one TestNG fork (98/98) and over a dirty database:
test_issue_496acceptsattributeOrValueExistswhen another backend test class already added the schema definitions to the shared in-JVM server, and the jdbcTestCasedrops staleopendj_%tables beforesetUp— PgSql and Encrypted map the same tree names to the same tables, and leftovers encrypted with a lost cipher key broke the next run's cleanup.