Skip to content

Conversation

@anishshri-db
Copy link
Contributor

@anishshri-db anishshri-db commented Sep 10, 2024

What changes were proposed in this pull request?

Throw RocksDB exception to the caller on DB close

API here: https://javadoc.io/doc/org.rocksdb/rocksdbjni/6.8.1/org/rocksdb/RocksDB.html#closeE()

Why are the changes needed?

Without this, errors on close are being silently ignored and we attempt to open the DB again in the context of the same task. Trying to ensure that we exit in this scenario and try as part of a separate attempt.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing unit tests

[info] Total number of tests run: 205
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 205, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.

Was this patch authored or co-authored using generative AI tooling?

No

@anishshri-db anishshri-db changed the title [SPARK-49593] Throw RocksDB exception to the caller on DB close if an error is seen [SPARK-49593][SS] Throw RocksDB exception to the caller on DB close if an error is seen Sep 10, 2024
@anishshri-db
Copy link
Contributor Author

cc - @siying , @HeartSaVioR - PTAL, thx !

Copy link
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

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

+1

@HeartSaVioR
Copy link
Contributor

https://github.com/anishshri-db/spark/actions/runs/10803458710/job/29973660998

All modules passed except Docker integration test, it's still running. This is unrelated to this change.

@HeartSaVioR
Copy link
Contributor

Thanks! Merging to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants