Skip to content

NoSQL: reduce heap pressure when running tests#3267

Merged
snazy merged 1 commit intoapache:mainfrom
snazy:nosql-reduce-test-heap-press
Jan 5, 2026
Merged

NoSQL: reduce heap pressure when running tests#3267
snazy merged 1 commit intoapache:mainfrom
snazy:nosql-reduce-test-heap-press

Conversation

@snazy
Copy link
Member

@snazy snazy commented Dec 14, 2025

Some tests generate a lot of realms, likely one realm per test case. While the amount of data per realm is not much, it is nontheless nice to remove that data immediately (for tests).

The maintenance service, which purges data of eligible realms, cannot be run against the in-memory backend (different JVM).

This change adds a rather "test only" workaround to purge the realm data in the in-memory backend immediately.

Some tests generate a lot of realms, likely one realm per test case. While the amount of data per realm is not much, it is nontheless nice to remove that data immediately (for tests).

The maintenance service, which purges data of eligible realms, cannot be run against the in-memory backend (different JVM).

This change adds a rather "test only" workaround to purge the realm data in the in-memory backend immediately.

if (updated.status() == PURGED) {
realmManagement.delete(updated);
if (updated.status() == PURGING || updated.status() == PURGED) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need the complex state transition sequence (via INACTIVE) with a loop now? Why not transition to the final state in one step?

Copy link
Member Author

Choose a reason for hiding this comment

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

Realm-managment defines possible realm-states and allowed transitions between those.
The intent, in this "purge realm" case, is to later allow a workflow to first set a realm to INACTIVE (aka it's still there, but you cannot use it), allowing users to revert their decision within a couple of days, pretty much what people can do in other SaaS offerings. From INACTIVE it can transition to PURGING, which means that the management-service takes care of actually deleting all realm data and eventually delete the realm definition itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I understand the current state transitions :) however, since in this piece of code the intent is to transition from any non-purged state to PURGING, having state transition restrictions in RealmManagementImpl seems artificial. This code will transition to the state it wants anyway, so why impede this transition and make it go through a loop?

This is a tangential comment based on general observations... not a blocker for this PR :)

Comment on lines +215 to +222
if (result.status() == PURGING && "InMemory".equals(backend.type())) {
// Handle the test/development case when using the in-memory backend:
// In this case there is no chance to run the maintenance service. To remove no longer
// necessary data from the Java heap, call the in-memory backend directly.
// This is "nice behavior" when running tests.
// The only "cost" is that the state PURGING may never be pushed to PURGED, but that is
// acceptable.
backend.deleteRealms(Set.of(realmId));
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks awkward... Could we delegate that to Backend impl?... or maybe move the special case to PolarisIntegrationTestFixture?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is a bit awkward, true. OTOH, InMemory is the only backend that doesn't persist anything, so we cannot use the management-service or even the admin-tool here.
PolarisIntegrationTestFixture can't be used in all tests, as it's only for Quarkus tests.

This change actually also applies when people try Polaris server w/ the InMemory backend.

Copy link
Contributor

@dimas-b dimas-b Dec 23, 2025

Choose a reason for hiding this comment

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

but as far as I can tell PolarisIntegrationTestFixture is the only piece of code that calls purge in tests... no other tests or user-driven actions will get to backend.deleteRealms() on this line... or did I miss something 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

You did ;)
There are tests in a lot of other modules, polaris-nosql-nosql-metastore and many other modules that do not "know" about those types.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, fair enough :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks :)

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jan 5, 2026
@snazy snazy merged commit a28abce into apache:main Jan 5, 2026
15 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Jan 5, 2026
@snazy snazy deleted the nosql-reduce-test-heap-press branch January 9, 2026 10:28
evindj pushed a commit to evindj/polaris that referenced this pull request Jan 26, 2026
Some tests generate a lot of realms, likely one realm per test case. While the amount of data per realm is not much, it is nontheless nice to remove that data immediately (for tests).

The maintenance service, which purges data of eligible realms, cannot be run against the in-memory backend (different JVM).

This change adds a rather "test only" workaround to purge the realm data in the in-memory backend immediately.
snazy added a commit to snazy/polaris that referenced this pull request Feb 11, 2026
* NoSQL: reduce heap pressure when running tests (apache#3267)

Some tests generate a lot of realms, likely one realm per test case. While the amount of data per realm is not much, it is nontheless nice to remove that data immediately (for tests).

The maintenance service, which purges data of eligible realms, cannot be run against the in-memory backend (different JVM).

This change adds a rather "test only" workaround to purge the realm data in the in-memory backend immediately.

* fix(deps): update dependency org.projectnessie.cel:cel-bom to v0.6.0 (apache#3356)

* chore: Suppress unchecked case from mock (apache#3322)

This is to fix javac `warning: [unchecked] unchecked conversion TriConsumer<String, TableIdentifier, MetricsReport> mockConsumer = mock(TriConsumer.class);`

* Add Polaris Console on the tools set (apache#3355)

* Move remaining build-time server properties to runtime/defaults (apache#3341)

The `runtime/defaults` module is meant as a holder of default
Polaris Server runtime properties.

This change moves a few remaining properties from the `application.properties`
file under `runtime/server` into `runtime/defaults` to avoid any possible
confusion regarding the location of effective server properties.

Note: The Admin tool has its own `application.properties` file.

* Deprecate untyped `RealmConfig.getConfig()` (apache#3323)

* `getConfig(String)` has a generic return type, but the call path that gets the value does not perform any type validation.

* Deprecate this method in favour of well-typed `getConfig(PolarisConfiguration)`

* Migrate the single use case in Polaris code to the well-typed method.

* fix(deps): update dependency io.smallrye.common:smallrye-common-annotation to v2.15.0 (apache#3365)

* Fix error handler parameters in TaskExecutorImpl (apache#3358)

Use correct exception variable inside `CompletableFuture.exceptionallyComposeAsync()`

* fix(deps): update dependency org.junit:junit-bom to v5.14.2 (apache#3363)

* fix(deps): update dependency com.gradleup.shadow:shadow-gradle-plugin to v9.3.1 (apache#3361)

* Propagate previous task exceptions as "suppressed" (apache#3367)

* Propagate previous task exceptions as "suppressed"

Task retries may fail in different ways in each attempt, however
only the last exception used to be exposed to the caller.

This change propagates exceptions from all previous tasks execution
attempts as "suppressed" exceptions chained to the final tasks
failure exception.

* Introduce idempotency records schema and Postgres-backed IdempotencyStore (apache#3205)

This PR adds the persistence foundation for REST idempotency in Polaris:
Defines an idempotency_records table (Postgres) with key, binding, liveness, and finalization fields.
Introduces a storage‑agnostic IdempotencyRecord model and IdempotencyStore SPI in polaris-core.
Implements PostgresIdempotencyStore in polaris-relational-jdbc

* fix(deps): update dependency ch.qos.logback:logback-classic to v1.5.24 (apache#3369)

* fix(deps): update immutables to v2.12.1 (apache#3368)

* Remove unnecessary version spec in jdbc persistence build file (apache#3373)

The testcontainers BOM is already included, so the version spec, which doesn't match the BOM version, is unnecessary.

* Fix typo (apache#3376)

* Cosmetic: sort lines in libs.versions.toml (apache#3133)

* fix(deps): update dependency com.github.dasniko:testcontainers-keycloak to v4.1.0 (apache#3375)

* Remove Admin tests from required_status_checks (apache#3370)

* fix(deps): update quarkus platform and group to v3.30.6 (apache#3374)

* [Bug] Fix a bug that causing error when setting `write.data.path` to be a subdirectory of the table location (apache#3371)

Currently, when updating write.data.path of the table to a subdir under the table location, it will fail the location overlap check. For example

spark-sql> ALTER TABLE tb1 SET TBLPROPERTIES (
  'write.data.path' = '<tableLocation>/alternative_data'
);

org.apache.iceberg.exceptions.ForbiddenException: Forbidden: Unable to create table at location 's3://<table_location>' because it conflicts with existing table or namespace at location 's3://<table_location>`
IcebergCatalog.validateNoLocationOverlap(...) constructs a virtual PolarisEntity for overlap checking, but it did not set the entity name. When fetching the siblings of the table, it fails to filter out itself and thus the check mistaken considered that the write.data.path conflict with the table's own base location. (isChildOf)

This PR fix the issue by adding name to the virtual PolarisEntity and add a unit and a integration test.

* Update release guide to reference the proposed vote e-mail (apache#3377)

Now that apache#3150 has been merged, a VOTE e-mail is pre-generated by the
third workflow so that there is less room for error.  The release guide
has been updated to reflect this.

There is no pre-generated Incubator vote e-mail as part of the third
workflow.  This is acceptable in that eventually, after graduation, that
would become unnecessary.  But also that it could cause confusion as
there would be two proposed e-mail bodies in the third workflow.
Recommendation is to let the release manager created the Incubator vote
e-mail manually.

* Last merged commit b324fc3

---------

Co-authored-by: Mend Renovate <[email protected]>
Co-authored-by: Dmitri Bourlatchkov <[email protected]>
Co-authored-by: JB Onofré <[email protected]>
Co-authored-by: Huaxin Gao <[email protected]>
Co-authored-by: Yong Zheng <[email protected]>
Co-authored-by: Yufei Gu <[email protected]>
Co-authored-by: Honah (Jonas) J. <[email protected]>
Co-authored-by: Pierre Laporte <[email protected]>
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.

2 participants