Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Memory leak in db.getMany() #192

Closed
tzapzoor opened this issue Mar 24, 2022 · 4 comments · Fixed by #193
Closed

Memory leak in db.getMany() #192

tzapzoor opened this issue Mar 24, 2022 · 4 comments · Fixed by #193
Labels
bug Something isn't working

Comments

@tzapzoor
Copy link

tzapzoor commented Mar 24, 2022

Context:
OS: macOS 12.2.1
Chip: Apple M1 Pro
Node: v16.13.2
rocksdb: 5.2.0
level-rocksdb: 5.0.0

Db options:

const level = require('level-rocksdb');

const db = level('./database', {
    valueEncoding: 'json',
    createIfMissing: false,
    readOnly: true,
});

I stress tested the db.GetMany method with batches of 500 keys as part of a simple node.js express app and after some time the memory usage of process was about 40GB and growing.

let keys: Buffer[] = getKeys(req.keys);
let values: (Value | undefined)[] = await db.getMany(keys);

I then replaced getMany with get and the memory stabilised at about 300MB, with worse performance, of course.

let values: (Value | undefined)[] = await Promise.all(
keys.map(async (k): Promise<(Value | undefined)> => {
    try {
        return await db.get(k);
    } catch(e) {
        return undefined;
    }
}));

The problem reproduces 100%. Do you guys have ideas why this could happen?

cc: @vweevers pinging as I saw you added support for getMany.

Thank you!

@vweevers
Copy link
Member

Could you write a simple (JS not TS) script to reproduce?

@vweevers vweevers added the bug Something isn't working label Mar 24, 2022
@vweevers vweevers changed the title [bug] Memory leak in db.getMany Memory leak in db.getMany() Mar 24, 2022
@tzapzoor
Copy link
Author

const level = require('level-rocksdb');

let db = level('./database', { valueEncoding: 'json' });

const getKeys = () => {
    let keys = [];

    for (let i = 0; i < 2000; i++) {
        keys.push('a');
    }

    return keys;
};

(async () => {
    await db.open();

    while (true) {
        let keys = getKeys();
        let values = await db.getMany(keys);
        console.log(values.length);
    }
})();

This can be run on an empty database as well. It fills 5GB in 2 minutes and growing.

@vweevers
Copy link
Member

Thanks, looking into it. Also reproduces on other implementations (that share code):

const { ClassicLevel } = require('classic-level')

const db = new ClassicLevel('./db/' + Date.now())
const keys = new Array(2000).fill(0).map((_, i) => String(i))

;(async () => {
  while (true) {
    await db.getMany(keys)
    console.log(process.memoryUsage().rss / 1024 / 1024)
  }
})()

@vweevers vweevers added this to Level Mar 24, 2022
@vweevers vweevers moved this to Backlog in Level Mar 24, 2022
@vweevers vweevers moved this from Backlog to In Progress in Level Mar 24, 2022
vweevers added a commit to Level/classic-level that referenced this issue Mar 24, 2022
vweevers added a commit to Level/leveldown that referenced this issue Mar 24, 2022
vweevers added a commit that referenced this issue Mar 24, 2022
@vweevers vweevers moved this from In Progress to Review in Level Mar 24, 2022
vweevers added a commit to Level/leveldown that referenced this issue Mar 24, 2022
vweevers added a commit to Level/leveldown that referenced this issue Mar 25, 2022
vweevers added a commit that referenced this issue Mar 25, 2022
vweevers added a commit to Level/classic-level that referenced this issue Mar 25, 2022
vweevers added a commit that referenced this issue Mar 25, 2022
Repository owner moved this from Review to Done in Level Mar 25, 2022
@vweevers
Copy link
Member

Fixed in 5.2.1. Thanks for the report - and your Open Collective donation ❤️

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants