Trying to support this is unnecessary and just adds branches and bit ops
when we could just round the allocation up or down
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#2497
# Note: this pull request is built on top of #2491 so let's merge that
one first.
## Problem:
A very easy source of bugs is to mistakenly use e.g.
`PageContent::read_u16()` instead of
`PageContent::read_u16_no_offset()`. The difference between the two is
that `read_u16()` adds 100 bytes to the requested byte offset if and
only if the page in question is page 1, which contains a 100-byte
database header.
Case in point about this being easy to misuse: see #2491.
## Observation:
In all of the cases where we want to read from or write to a page
"header-sensitively" (taking the possible db header into account), those
reads/writes are to statically known offsets, e.g. specific known bytes
in a btree page header.
In all other cases, the "no-offset" versions, i.e. the ones taking the
absolute byte offset as parameter, should be used - the common use cases
for these are reading/writing to some absolute offset that you just read
from another location on a page, for example. Another use case is
writing to specific offsets on overflow pages and freelist pages, which
we can in the future expose more methods for, but right now they can
always use the "no offset" versions safely, because neither of those
page types can ever be page 1.
## Solution:
1. Make all the offset-sensitive versions (`read_u16()` and friends)
private methods of `PageContent`.
2. Expose dedicated public methods for things like updating rightmost
pointer, updating fragmented bytes count and so on, and use them instead
of the plain read/write methods universally.
## Follow-up:
I will perhaps follow this up with a renaming PR that renames
`read_xxx_no_offset()` to just `read_xxx()` and gives the private
header-sensitive methods new names.
Reviewed-by: Preston Thorpe <preston@turso.tech>
Closes#2493
Problem:
A very easy source of bugs is to mistakenly use e.g. PageContent::read_u16()
instead of PageContent::read_u16_no_offset(). The difference between the two
is that `read_u16()` adds 100 bytes to the requested byte offset if and only if
the page in question is page 1, which contains a 100-byte database header.
Case in point: see #2491.
Observation:
In all of the cases where we want to read from or write to a page "header-sensitively",
those reads/writes are to so-called "well known offsets", e.g. specific bytes in a btree
page header.
In all other cases, the "no-offset" versions, i.e. the ones taking the absolute byte offset
as parameter, should be used.
Solution:
1. Make all the offset-sensitive versions (read_u16() and friends) private methods of
`PageContent`.
2. Expose dedicated methods for things like updating rightmost pointer, updating fragmented
bytes count and so on, and use them instead of the plain read/write methods universally.
## Beef
`defragment_page_fast()` incorrectly didn't use the version of
read/write methods on `PageContent` that does NOT add the 100 byte
database header into the requested byte offset.
this resulted in defragment of page 1 in reading 2nd/3rd freeblocks
from the wrong offset and reading/writing freeblock sizes and cell
offsets to the wrong location.
## Testing
Adds fuzz test for CREATE TABLE / DROP TABLE / ALTER TABLE, which I was
able to reproduce this with.
Closes#2491
`defragment_page_fast()` incorrectly didn't use the version of
read/write methods on `PageContent` that does NOT add the 100 byte
database header into the requested byte offset.
this resulted in defragment of page 1 in reading 2nd/3rd freeblocks
from the wrong offset and writing cell offsets to the wrong location.
synchronous=FULL means WAL is fsynced on every commit, which is what we
also do.
however we do not do the padding to sector alignment that sqlite does
(see #2450), which makes sqlite do more work than we do.
Closes#2485
synchronous=FULL means WAL is fsynced on every commit,
which is what we also do.
however we do not do the padding to sector alignment that
sqlite does (see #2450), which makes sqlite do more work
than we do.
Closes#2478
```console
turso>create table t(a,b);
turso>insert into t select 1,2 from generate_series(1,20);
turso>create index idxa on t(a);
turso>create index idxb on t(b);
# segfault
```
The issue was the `turso_ext::Conn` pointer was stored on the
`VirtualTable`, which lives longer than necessary and the underlying
core `Connection` is not guaranteed pinned. Moving the `turso_ext::Conn`
on to the cursor fixes this issue because it's independent of the
Schema.
This also fixes the issue that comes up when that issue is fixed, and
some of the relevant code for this hadn't been updated when other
surrounding changes had been made.
Closes#2479
This PR rewrites `turso-sync` package introduced in the #2334 and
renames it to the `turso-sync-engine` (anyway the diff will be
unreadable).
The purpose of rewrite is to get rid of Tokio because this makes things
harder when we wants to export bindings to WASM.
In order to achieve "runtime"-agnostic sync core but still be able to
use async/await machiner - this PR introduce usage of `genawaiter` crate
which allows to transfer async/await Rust state machines to the
generators. So, sync operations are just generators which can yield `IO`
command in case where there is a need for it.
Also, this PR introduces separate `ProtocolIo` in the `turso-sync-
engine` which defines extra IO methods:
1. HTTP interaction
2. Atomic read/writes to the file. This is not strictly necessary and
`turso_core::IO` methods can be extended to support few more things
(like `delete`/`rename`) - but I decided that it will be simpler to just
expose 2 more methods for sync protocol for the sake of atomic metadata
update (which is very small - dozens of bytes).
* As a bonus, we can store metadata for browser in the
`LocalStorage` which may be more natural thing to do(?) (user can reset
everything by just clearing local storage)
The `ProtocolIo` works similarly to the `IO` in a sense that it gives
the caller `Completion` which it can check periodically for new data.
Closes#2457
**86%** performance improvement. We are **25x** faster than SQLite.
<img width="953" height="511" alt="image" src="https://github.com/user-
attachments/assets/fd717d1e-bbbe-4959-ae48-41afc73e5e9f" />
```
ALTER TABLE _ DROP COLUMN _`/limbo_drop_column/
time: [1.8821 ms 1.8929 ms 1.9047 ms]
change: [-86.850% -86.733% -86.614%] (p = 0.00 < 0.05)
Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
3 (3.00%) high mild
2 (2.00%) high severe
Benchmarking `ALTER TABLE _ DROP COLUMN _`/sqlite_drop_column/: Warming up for 3.0000 s
`ALTER TABLE _ DROP COLUMN _`/sqlite_drop_column/
time: [46.227 ms 46.258 ms 46.291 ms]
change: [-1.3202% -1.0505% -0.8109%] (p = 0.00 < 0.05)
Change within noise threshold.
Found 15 outliers among 100 measurements (15.00%)
10 (10.00%) high mild
5 (5.00%) high severe
```
Closes#2452
## Problem:
Currently `WriteState`, usually triggered by an insert operation, "owns"
the balancing state machine, even though a delete operation (tracked by
a separate `DeleteState`) can also trigger balancing, which results in
awkward back-and-forth switching between `CursorState::Write` and
`CursorState::Delete` during balancing.
## Fix:
1. Extract `balance_state` as a separate state machine, since its state
transitions are exactly the same regardless of whether an insert or a
delete triggered the balancing.
2. This allows to remove the different 'Balance-xxx' variants from
`WriteState`, as well as removing `WriteInfo` and `DeleteInfo`, as the
delete&insert states become just simple enums now. Each of them now has
a substate called `Balancing` which just delegates work to the balancing
state machine.
3. This further allows us to remove the awkward switching between
`CursorState::Delete` and `CursorState::Write` during a balance that
happens as a result of a deletion.
Reviewed-by: Nikita Sivukhin (@sivukhin)
Reviewed-by: Avinash Sajjanshetty (@avinassh)
Closes#2468
