[OPENJPA-2963] Track dropped tables per database, not per JVM - #179
Open
rzo1 wants to merge 1 commit into
Open
Conversation
The dropped-table tracking was one JVM-global set, and clearDroppedTables() is called whenever any factory with schema generation properties starts up, so two persistence units initializing at once wiped each other's in-flight tracking. Keys were bare table names too, so two units on different databases with a table of the same name aliased onto one entry. The set is now partitioned by the database a configuration connects to, and the clear only empties that database's entry. The state stays static: it has to outlive the configuration that wrote it, because generateSchema() closes its own factory and a later factory is expected to see what it dropped, which TestSchemaGenDrop pins. Moving it onto the configuration, as the issue suggests, would leave it with no readers and break that test. Every access now holds one monitor, including the trace line, which previously interpolated the live set while another thread could be writing to it. The locale half of the issue no longer applies: 53269ab already gave every one of those call sites Locale.ROOT, and the line numbers in the issue are from before it. Not addressed here, and worth their own issues: the DDL name matching still compares a regex-stripped fragment of the raw statement against a schema-qualified identifier, so it misses and over-matches in several shapes; and it is claimed that a drop-and-create with a script drop source suppresses the create it is paired with, which I could not reproduce.
Contributor
|
Could you provide an unit test for the issue? It would help to avoid future regressions. |
solomax
reviewed
Sep 3, 2026
| */ | ||
| private static String databaseKey(JDBCConfiguration conf) { | ||
| String[] candidates = new String[] { | ||
| conf.getConnectionFactory2Name(), conf.getConnection2URL(), |
Contributor
There was a problem hiding this comment.
I was unable to find any proof getConnectionFactory2Name/getConnectionFactoryName is unique
Do you have one? :)
Maybe it worth to use Configuration.getId which is mapped to JPA persistence-unit name ?
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.
The dropped-table tracking was one JVM-global set, and
clearDroppedTables()is called whenever any factory with schema-generation properties starts up, so two persistence units initializing at once wiped each other's in-flight tracking. Keys were bare table names too, so two units on different databases with a table of the same name aliased onto one entry.The set is now partitioned by the database a configuration connects to, and the clear empties only that database's entry. Every access holds one monitor, including the trace line, which previously interpolated the live set while another thread could be writing to it. The no-arg
clearDroppedTables()is kept and deprecated.On the suggestion to move the state onto the configuration: that is not available. The tracking has to outlive the configuration that wrote it —
Persistence.generateSchema()closes its own factory, and a later, separately created factory is expected to see what it dropped, whichTestSchemaGenDrop.testDropViaGenerateSchemapins. Config-scoped, the set would have almost no readers and that test would fail. So the state stays static and is scoped instead.On the locale half of the issue: it no longer applies. 53269ab already gave every one of those call sites
Locale.ROOT; the line numbers in the issue are from before it.Deliberately not addressed here, each worth its own issue:
DROP TABLE IF EXISTS FOOyields an empty key;DROP TABLE ORDER_CASCADE_LOGyieldsORDER_) and over-matches. Fixing it in isolation would make things worse rather than better: more names would match, so any suppression defect would fire in more configurations than it does today.drop-and-createwith a script drop source suppresses the create it is paired with, because both run in one action list against one configuration. I wrote the fix and a test for it, and the test passed against unpatched master — with an explicitdrop-source=scriptand with a verifying factory that has noSynchronizeMappingsand so cannot create the table itself. I could not reproduce it, so the fix is not included.There is no new test. The concurrency defect is not deterministically testable, and the one testable claim did not reproduce; the change rests on the existing schema-generation suite (35 tests, passing).