zhaomin1423 commented on PR #43502:
URL: https://github.com/apache/spark/pull/43502#issuecomment-1778423067
> @zhaomin1423
>
> Firstly, we may need to add two helper methods:
>
> 1. Add a method in `RocksDB` that can get its corresponding
`WeakReference` through `RocksDBIterator`, here I assume it's called
`iteratorReference`
> 2. Add a method in `RocksDBIterator` to get `RocksIterator`, here I assume
it's called `internalIterator`
>
> Then, the test case may like the following:
>
> ```java
> @Test
> public void testResourceCleaner() throws Exception {
> File dbPathForCleanerTest = File.createTempFile(
> "test_db_cleaner.", ".rdb");
> dbPathForCleanerTest.delete();
>
> RocksDB dbForCleanerTest = new RocksDB(dbPathForCleanerTest);
> for (int i = 0; i < 8192; i++) {
> dbForCleanerTest.write(createCustomType1(i));
> }
>
> RocksDBIterator<CustomType1> rocksDBIterator =
(RocksDBIterator<CustomType1>)
dbForCleanerTest.view(CustomType1.class).iterator();
> Optional<Reference<RocksDBIterator<?>>> referenceOpt =
dbForCleanerTest.iteratorReference(rocksDBIterator);
> assertTrue(referenceOpt.isPresent());
> RocksIterator it = rocksDBIterator.internalIterator();
> assertTrue(it.isOwningHandle()); // it has not been closed yet,
isOwningHandle should be true.
> rocksDBIterator = null; // Manually set rocksDBIterator to null, to be
GC.
> Reference<RocksDBIterator<?>> ref = referenceOpt.get();
> // 100 times gc, the rocksDBIterator should be GCed.
> int count = 0;
> while (count < 100 && !ref.refersTo(null)) {
> System.gc();
> count++;
> Thread.sleep(100);
> }
> assertTrue(ref.refersTo(null)); // check rocksDBIterator should be GCed
> // Verify that the Cleaner will be executed after a period of time, and
it.isOwningHandle() will become false.
> assertTimeout(java.time.Duration.ofSeconds(5), () ->
assertFalse(it.isOwningHandle()));
>
> dbForCleanerTest.close();
> }
> ```
>
> Of course, we can also add a state in `ResourceCleaner` to indicate
whether it has been executed, initially set to false, and turns true after
execution. These are just some of my thoughts, there might be better ways to
test.
Adding a state is simpler and works for different classes, but I don't know
if adding variables for testing is a common approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
