Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Subleveldown is now unnecessary, and it can be replaced with keypaths #12

Merged
merged 2 commits into from
Mar 27, 2022

Conversation

CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Mar 26, 2022

Description

By getting rid of subleveldown, the API of DB changes quite significantly but also much more simpler:

// root iterator at dataDb
DB.iterator(options);
// same
DB.iterator(options, []);
// down level 1
DB.iterator(options, ['level1']);
DB.iterator(options, ['level1', 'level2]);
DBTransaction.iterator(options, ['level1', 'level2']);

DB.count();
DB.count(['level1', 'level2']);
DBTransaction.count();
DBTransaction.count(['level1']);

DB.clear();
DB.clear(['level1']);
DBTransaction.clear(['level1']); // this should be a transactional clear, to do this, we must iterator and then do `db.del`

DB.get('key');
DB.get(['level1', 'key']);
// these are not allowed, or they are equivalent to asking for undefined
DB.get([]);
DB.get();

DB.put('key', 'value');
DB.put(['level1', 'key'], 'value');

DB.del('key');
DB.del(['level1', 'key']);
DBTransaction.del('key');
DBTransaction.del(['level1', 'key']);

Firstly we won't have to deal with creating sublevels in an inefficient way: #11 (comment). Instead we just do everything through key concatenation.

We still need a separator which we can preserve as ! as the default.

For iteration, we would need to use #11 (comment)

iterator({
  // gt means greater than this key
  // since !A!!B! would be the domain path, any key added would always be 1 character more
  gt: '!A!!B!',
  // this would have to be the very next byte number above !A!!B!, but that which doesn't produce an extra character, we could create a buffer and just increment by 1 for the the last byte
  // if `!` is the separator which is 0x21, then plus 1 would be `"` which is 0x22
  lt: '!A!!B"'
})

This also means all downstream packages that use DB will only just use the DB instance, they don't bother creating sublevel instances for no reason. There's no need to keep around sublevel instances anymore. This also reduces one headache which is that you have to "recreate" the sublevels whenever you stop the underlying database.

See how in our destroy methods such as SessionManager it requires:

  public async destroy() {
    this.logger.info(`Destroying ${this.constructor.name}`);
    const sessionsDb = await this.db.level(this.sessionsDbDomain);
    await sessionsDb.clear();
    this.logger.info(`Destroyed ${this.constructor.name}`);
  }

That's just unnecessary now.

You can just do:

  public async destroy() {
    this.logger.info(`Destroying ${this.constructor.name}`);
    await db.clear([this.sessionsDbDomain]);
    this.logger.info(`Destroyed ${this.constructor.name}`);
  }

This will probably mean a change to how we do our db domains, we just need to keep track of the levels we are operating on and that's it.

Issues Fixed

Tasks

  • 1. Removed subleveldown and ancillary dependencies
  • 2. Updated all signatures to use KeyPath and LevelPath
  • 3. Aligned the iterator to be synchronous for both DBTransaction and DB

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Mar 26, 2022

The domainPath function differentiates keys from paths.

When using the domain path, we now have to consider what is the user trying to do.

// this means sublevel of A then B
await db.iterator(options, ['A', 'B']);
// does this mean sublevel of A, or just A the key?
// it has to be sublevel of A obviously, since if the key is the only thing, then use the options parameter
await db.iterator(options, ['A']);

// does this mean get A the key?
// or does it mean get A the sublevel?
// it would have to mean A the key, since you cannot get a sublevel
await db.get(['A']);

await db.get(['A', 'B']);

This means we should differentiate DomainPath from KeyPath.

A KeyPath means that the last element is always the final key.

A DomainPath means that the last element is still a sublevel.

@CMCDragonkai
Copy link
Member Author

Therefore:

get(keyPath: KeyPath)
put(keyPath: KeyPath)
del(keyPath: KeyPath)

iterator(options, domainPath: DomainPath)
count(domainPath: DomainPath)
clear(options, domainPath: DomainPath)
dump(domainPath: DomainPath)

We can get rid of batch since that's done by Transaction now.

@CMCDragonkai
Copy link
Member Author

Might prefer LevelPath over DomainPath as it is more precise, and specific to leveldb, but even other ordered key values can match the same idea.

@CMCDragonkai
Copy link
Member Author

Additionally we can say LevelPath can be empty to indicate the root level, while KeyPath has to be filled as you must refer to a single key at the very least.

@CMCDragonkai
Copy link
Member Author

We now have:

keyPathToKey: KeyPath => Buffer
levelPathTokey: LevelPath => Buffer
parseKey: Buffer => KeyPath

This allows us to get rid of sublevel-prefixer, and we have a much more easier to understand code here.

@CMCDragonkai
Copy link
Member Author

The default separator is changed 0x00 which means the 1 byte above is 0x01. These are unprintable characters in ASCII, so when key path are dumped using binary encoding, we will see \x00 and \x01. This frees up the !` the likelihood of clashes becomes less likely for string sublevels.

If we were really intending to allow sublevels to be arbitrary buffers, we would also need to do escaping.

But this complicates things in DB iteration, and possibly one was to use order-preserving base encoding on all sublevels to avoid that one character of 0x00.

For now buffers can be used in level paths, but cannot be arbitrary buffers, or at least cannot contain the separator buffer of 0x00.

@CMCDragonkai
Copy link
Member Author

In the process of reworking the iterator, I realised I had a bug in the the current master where the keys and values properties were typed as key and value for iterator method.

@CMCDragonkai
Copy link
Member Author

This test is now no longer relevant and is removed:

  test('async start and stop requires recreation of db levels', async () => {
    const dbPath = `${dataDir}/db`;
    const db = await DB.createDB({ dbPath, logger });
    await db.start();
    let level1 = await db.level('level1');
    await db.put(['level1'], 'key', 'value');
    await db.stop();
    await db.start();
    // The `level1` has to be recreated after `await db.stop()`
    await expect(db.level('level2', level1)).rejects.toThrow(
      /Inner database is not open/,
    );
    level1 = await db.level('level1');
    await db.level('level2', level1);
    expect(await db.get(['level1'], 'key')).toBe('value');
    await db.stop();
  });

@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Mar 27, 2022

We will need https://www.typescriptlang.org/tsconfig/#stripInternal as I now have "internal methods" that are shared between DB and DBTransaction, but lacking "friend" classes, these are public methods but they will be stripped.

https://stackoverflow.com/questions/45847399/friend-class-in-typescript

@CMCDragonkai CMCDragonkai changed the title WIP: Subleveldown is now unnecessary, and it can be replaced with keypaths Subleveldown is now unnecessary, and it can be replaced with keypaths Mar 27, 2022
@CMCDragonkai
Copy link
Member Author

This is ready to be merged now. All tests pass, and benchmarks have been re-ran.

The dependencies have been significantly pruned here.

@CMCDragonkai CMCDragonkai self-assigned this Mar 27, 2022
@CMCDragonkai CMCDragonkai merged commit dcf70c2 into master Mar 27, 2022
@CMCDragonkai CMCDragonkai deleted the nosubleveldown branch March 27, 2022 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Integrate DB into the level db interface so db levels are the same as db
1 participant