Skip to content

Conversation

@fm3
Copy link
Member

@fm3 fm3 commented Aug 19, 2024

The caller should close the Iterator returned by RocksDB.newIterator. That’s us.

I ran this stress test

    for i in range(100000):
        reply = stub.ListVersions(proto.ListVersionsRequest(collection=collection, key=volume_id))
        assert_success(reply)

and on master, fossilDB memory usage went up by about 1.5 GB for each run. On this branch, it stayed steady even after multiple runs. Speed seems unaffected (variations are smaller than noise)

@fm3 fm3 self-assigned this Aug 19, 2024
@fm3 fm3 marked this pull request as ready for review August 19, 2024 15:30
@fm3 fm3 requested a review from normanrz August 19, 2024 15:30
@fm3 fm3 changed the title Close all iterators Close all RocksIterators Aug 19, 2024
Copy link
Member

@normanrz normanrz left a comment

Choose a reason for hiding this comment

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

Good stuff!

@fm3 fm3 merged commit d851fd3 into master Aug 20, 2024
@fm3 fm3 deleted the close-all-iterators branch August 20, 2024 09:56
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.

3 participants