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.

Sign up for GitHub

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

Jump to bottom

Epic: implicit and explicit snapshots #118

Open
6 of 11 tasks
vweevers opened this issue Sep 26, 2022 · 0 comments
Open
6 of 11 tasks

Epic: implicit and explicit snapshots #118

vweevers opened this issue Sep 26, 2022 · 0 comments

Comments

@vweevers
Copy link
Member

vweevers commented Sep 26, 2022 •
edited
Loading

Implicit snapshots

Main task here is to fix the issue described in Level/leveldown#796.

Explicit snapshots

This is a new feature, explained by the first PR below. Prior discussions: #45, #47, Level/leveldown#486.
@vweevers vweevers added this to Level Sep 26, 2022
@vweevers vweevers moved this to Backlog in Level Sep 26, 2022
This was referenced Sep 26, 2022
Closed
Closed
Closed
Closed
@vweevers vweevers mentioned this issue Nov 4, 2022
Closed
15 tasks
vweevers added a commit to Level/abstract-level that referenced this issue Nov 10, 2022
@vweevers
Breaking: require snapshots to be created synchronously
d2e9ef4 
Ref Level/community#118.
@vweevers vweevers mentioned this issue Nov 10, 2022
Merged
vweevers added a commit to Level/abstract-level that referenced this issue Nov 10, 2022
@vweevers
Breaking: require snapshots to be created synchronously (#54)
03da0fe 
Ref Level/community#118.
vweevers added a commit to Level/classic-level that referenced this issue Nov 18, 2022
@vweevers
Create snapshot for get() synchronously
b95f5a9 
Has a tiny performance cost, which I negated by optimizing the
passing of options from JS to C++. The end result is faster than
before. However, I didn't check if it blocks the event loop for
a significant amount of time. Benchmarking concurrent gets might
answer that, later.

Ref Level/community#118.
@vweevers vweevers mentioned this issue Nov 19, 2022
Merged
12 tasks
vweevers added a commit to Level/abstract-level that referenced this issue Jan 27, 2024
@vweevers
Breaking: require snapshots to be created synchronously (#54)
d89e68e 
Ref Level/community#118.
vweevers added a commit to Level/classic-level that referenced this issue Feb 3, 2024
@vweevers
Create snapshot for get() synchronously
b4404b3 
Has a tiny performance cost, which I negated by optimizing the
passing of options from JS to C++. The end result is faster than
before. However, I didn't check if it blocks the event loop for
a significant amount of time. Benchmarking concurrent gets might
answer that, later.

Ref Level/community#118.
vweevers added a commit to Level/classic-level that referenced this issue Oct 20, 2024
@vweevers
Create snapshot for get() synchronously
fbcdb7b 
Has a tiny performance cost, which I negated by optimizing the
passing of options from JS to C++. The end result is faster than
before. However, I didn't check if it blocks the event loop for
a significant amount of time. Benchmarking concurrent gets might
answer that, later.

Ref: Level/community#118
Category: fix
vweevers added a commit to Level/classic-level that referenced this issue Oct 20, 2024
@vweevers
Create snapshot for get() synchronously
50e03dc 
Has a tiny performance cost, which I negated by optimizing the
passing of options from JS to C++. The end result is faster than
before. However, I didn't check if it blocks the event loop for
a significant amount of time. Benchmarking concurrent gets might
answer that, later.

Ref: Level/community#118
Category: fix
@vweevers vweevers mentioned this issue Oct 21, 2024
Closed
vweevers added a commit to Level/supports that referenced this issue Dec 21, 2024
@vweevers
Add implicit- and explicitSnapshots
68bac51 
Ref: Level/community#118
Category: addition
vweevers added a commit to Level/supports that referenced this issue Dec 21, 2024
@vweevers
Add implicit- and explicitSnapshots
e4b5378 
Ref: Level/community#118
Category: addition
vweevers added a commit to Level/abstract-level that referenced this issue Dec 22, 2024
@vweevers
Implement explicit snapshots
2b99040 
See Level/community#118. Depends on
Level/supports#32.

Category: addition
@vweevers vweevers mentioned this issue Dec 22, 2024
Merged
vweevers added a commit to Level/memory-level that referenced this issue Dec 22, 2024
@vweevers
Implement explicit snapshots
2bb79fa 
See Level/community#118. Depends on
Level/abstract-level#93.

Category: addition
@vweevers vweevers mentioned this issue Dec 22, 2024
Merged
@vweevers vweevers moved this from Backlog to In Progress in Level Dec 22, 2024
vweevers added a commit to Level/supports that referenced this issue Dec 23, 2024
@vweevers
Add implicit- and explicitSnapshots
61f15ae 
Ref: Level/community#118
Category: addition
vweevers added a commit to Level/abstract-level that referenced this issue Dec 23, 2024
@vweevers
Implement explicit snapshots
7dca766 
See Level/community#118. Depends on
Level/supports#32.

Category: addition
vweevers added a commit to Level/memory-level that referenced this issue Dec 23, 2024
@vweevers
Implement explicit snapshots
22e9e64 
See Level/community#118. Depends on
Level/abstract-level#93.

Category: addition
vweevers added a commit to Level/abstract-level that referenced this issue Dec 27, 2024
@vweevers
Implement explicit snapshots
7e97bc6 
See Level/community#118. Depends on
Level/supports#32.

Category: addition
vweevers added a commit to Level/classic-level that referenced this issue Dec 27, 2024
@vweevers
Implement explicit snapshots
c2a6c3d 
See Level/community#118. TLDR:

```js
await db.put('example', 'before')
const snapshot = db.snapshot()
await db.put('example', 'after')
await db.get('example', { snapshot })) // Returns 'before'
await snapshot.close()
```

This was relatively easy to implement because it's merely exposing an
existing LevelDB feature. Which we were already using too (for example,
creating an iterator internally creates a snapshot; I now refer to that
as an "implicit" snapshot). The only challenge was garbage collection
and state management. For that I knew I could copy iterator logic but I
wanted to avoid its technical debt, so I first cleaned that up:

- `binding.iterator_close()` is now synchronous, because that's 2x
  faster than `napi_create_async_work`, let alone executing that async
  work. Measured with `performance.timerify(binding.iterator_close)`.
  Simplifies state.
- I changed the database reference (`Database#ref_`) from a weak to a
  strong reference, preventing GC until `db.close()`. I thought this
  would remove the need for strong references to iterators & snapshots
  seeing as the `AbstractLevel` class already has references in the
  `db[kResources]` set, but a new unit test in `iterator-gc-test.js`
  proved me wrong. So I kept that as-is (would like to revisit).
- Having a strong reference to the database removed the need for the
  `IncrementPriorityWork` function to increment the refcount of said
  reference. It now just increments a `std::atomic<int>` (for our own
  state outside of V8).
- Iterators no longer call `IncrementPriorityWork` which had 2
  purposes:
  1. Increase the db reference count. See above.
  2. Delay database close until iterators were closed. A leftover from
     before `abstract-level` which closes iterators & snapshots before
     calling `db._close()`.

I then moved common logic for references and teardown to a new struct
called `Resource`, inherited by `Iterator` and `ExplicitSnapshot`. The
latter is a new struct that wraps a LevelDB snapshot.

Keeping snapshots open during read operations like `db.get()` is
handled in `abstract-level` through `AbstractSnapshot#ref()`.

I think further cleanup is possible (and to move more state management
to JS) but I'll save that for a future PR.

This PR depends on Level/abstract-level#93 but
is otherwise complete.
@vweevers vweevers mentioned this issue Dec 27, 2024
Open
vweevers added a commit to Level/abstract-level that referenced this issue Dec 27, 2024
@vweevers
Implement explicit snapshots (#93)
f81d348 
See Level/community#118. TLDR:

```js
await db.put('example', 'before')
const snapshot = db.snapshot()
await db.put('example', 'after')
await db.get('example', { snapshot })) // Returns 'before'
await snapshot.close()
```

Category: addition
@vweevers vweevers changed the title Tracking issue: implicit and explicit snapshots Epic: implicit and explicit snapshots Dec 27, 2024
vweevers added a commit to Level/abstract-level that referenced this issue Dec 29, 2024
@vweevers
Improve snapshot documentation
725e313 
- Add `snapshot` to TypeScript option types
- Remove redundant documentation from README
- Don't mark explicit snapshots as experimental. I'm confident in this
  feature now (having finished the implementation in `classic-level`)
  and I'm happy with the API.

Ref: Level/community#118
Follow-Up-To: #93
@vweevers vweevers mentioned this issue Dec 29, 2024
Merged
vweevers added a commit to Level/abstract-level that referenced this issue Dec 29, 2024
@vweevers
Improve snapshot documentation (#98)
b5b583c 
- Add `snapshot` to TypeScript option types
- Remove redundant documentation from README
- Don't mark explicit snapshots as experimental. I'm confident in this
  feature now (having finished the implementation in `classic-level`)
  and I'm happy with the API.

Ref: Level/community#118
Follow-Up-To: #93
vweevers added a commit to Level/memory-level that referenced this issue Jan 5, 2025
@vweevers
Implement explicit snapshots
98d0213 
See Level/community#118.

Category: addition
vweevers added a commit to Level/memory-level that referenced this issue Jan 5, 2025
@vweevers
Implement explicit snapshots (#16)
f832018 
See Level/community#118. TLDR:

```js
await db.put('example', 'before')
const snapshot = db.snapshot()
await db.put('example', 'after')
await db.get('example', { snapshot })) // Returns 'before'
await snapshot.close()
```

Category: addition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
Level
Status: In Progress
Milestone
No milestone
Development

No branches or pull requests
1 participant
@vweevers