No worries, I should have included a ";)" to let you know it was mostly
tongue-in-cheek.
Thanks,
-John
On 6/21/23 12:34, Nick Telford wrote:
Sorry John, I didn't mean to mis-characterize it like that. I was mostly
referring to disabling memtables. AFAIK the SstFileWriter API is primarily
designed for bulk ingest, e.g. for bootstrapping a database from a backup,
rather than during normal operation of an online database. That said, I was
overly alarmist in my phrasing.
My concern is only that, while the concept seems quite reasonable, there
are no doubt hidden issues lurking.
On Wed, 21 Jun 2023 at 18:25, John Roesler <vvcep...@apache.org> wrote:
Thanks Nick,
That sounds good to me.
I can't let (2) slide, though.. Writing and ingesting SST files is not a
RocksDB internal, but rather a supported usage pattern on public APIs.
Regardless, I think your overall preference is fine with me, especially
if we can internalize this change within the store implementation itself.
Thanks,
-John
On 6/21/23 11:50, Nick Telford wrote:
Hi Bruno,
1.
Isn't this exactly the same issue that WriteBatchWithIndex transactions
have, whereby exceeding (or likely to exceed) configured memory needs to
trigger an early commit?
2.
This is one of my big concerns. Ultimately, any approach based on
cracking
open RocksDB internals and using it in ways it's not really designed for
is
likely to have some unforseen performance or consistency issues.
3.
What's your motivation for removing these early commits? While not
ideal, I
think they're a decent compromise to ensure consistency whilst
maintaining
good and predictable performance.
All 3 of your suggested ideas seem *very* complicated, and might actually
make behaviour less predictable for users as a consequence.
I'm a bit concerned that the scope of this KIP is growing a bit out of
control. While it's good to discuss ideas for future improvements, I
think
it's important to narrow the scope down to a design that achieves the
most
pressing objectives (constant sized restorations during dirty
close/unexpected errors). Any design that this KIP produces can
ultimately
be changed in the future, especially if the bulk of it is internal
behaviour.
I'm going to spend some time next week trying to re-work the original
WriteBatchWithIndex design to remove the newTransaction() method, such
that
it's just an implementation detail of RocksDBStore. That way, if we want
to
replace WBWI with something in the future, like the SST file management
outlined by John, then we can do so with little/no API changes.
Regards,
Nick
