feat: Significantly decrease startup times for WAL #25643

mgattozzi · 2024-12-11T18:42:11Z

feat: add startup time to logging output

This change adds a startup time counter to the output when starting up
a server. The main purpose of this is to verify whether the impact of
changes actually speeds up the loading of the server.

feat: Significantly decrease startup times for WAL

This commit does a few important things to speedup startup times:

We avoid changing an Arc to a String with the series key as the
From impl will call with_column which will then turn it into
an Arc again. Instead we can just call with_column directly
and pass in the iterator without also collecting into a Vec
We switch to using bitcode as the serialization format for the WAL.
This significantly reduces startup time as this format is faster to
use instead of JSON, which was eating up massive amounts of time.
Part of this change involves not using the tag feature of serde as
it's currently not supported by bincode
We also parallelize reading and deserializing the WAL files before
we then apply them in order. This reduces time waiting on IO and we
eagerly evaluate each spawned task in order as much as possible.

This gives us about a 189% speedup over what we were doing before.

Closes #25534

hiltontj · 2024-12-11T19:24:06Z

Curious why you chose bitcode. One of their project non-goals is:

Stable format across major versions

i.e., they do not guarantee stability of the format across major versions. I know we have a bit of leeway with WAL files since they are only kept around for relatively short amount of time, but I think we would need to have some kind of versioning and upgrade process in place if we ever need to update our bitcode version, based on that statement.

If any of the other serde-compatible formats offer stability and similar performance gains, it might be worth considering those (Avro, MSGpack, etc.).

mgattozzi · 2024-12-11T20:55:47Z

Bitcode was the fastest according to these benchmarks which was the main draw. I think we can get around versioning issues by adding a version into the header that we check already to verify the wal is okay and then check which version to use to deserialize the code. We would need this also for breaking changes to the file format anyways if we wanted to do that and handle it without the user needing to know

hiltontj · 2024-12-11T21:25:47Z

Bitcode was the fastest according to these benchmarks which was the main draw.

Seems like others that perform similarly in their benchmarks are a bit too obscure.

We would need this also for breaking changes to the file format anyways if we wanted to do that and handle it without the user needing to know

That is true. Also, looking at some others, e.g., bincode and apache_avro, it seems like they have similar statements about stability, so something we probably need to handle regardless of the format we choose.

hiltontj

I had one comment on the profile change. Otherwise, the only thing to mention is that it would be good to track an issue for versioning the WAL file in the header.

hiltontj · 2024-12-11T21:38:12Z

Cargo.toml

@@ -182,6 +183,7 @@ inherits = "release"
 codegen-units = 16
 lto = false
 incremental = true
+debug = 1


I think that the quick-bench below provides this profile, i.e., quick-release+debug=1, would you be okay to leave this out in favour of that?

oh yeah no problem I meant to take this out

praveen-influx

Looks good, maybe one for the future - is there any zero copy alternatives that could be used for specifically serializing/deserializing wal?

pauldix · 2024-12-12T14:53:32Z

@praveen-influx Flatbuffers would be the thing, but I've found those generally very difficult to work with and not usually worth the effort. But we could certainly prototype it at some point in the future. But I don't think it's worth doing before the alpha.

mgattozzi · 2024-12-12T16:17:33Z

@praveen-influx and @pauldix there's rkyv for zero copy which I've been keeping an eye on for awhile

https://docs.rs/rkyv/latest/rkyv/

The developer of it did a talk at RustConf this year as well -> https://www.youtube.com/watch?v=ON4z2LbTD-4

mgattozzi · 2024-12-12T16:19:03Z

@hiltontj looks like we already have the version in the header! No need for a new issue

influxdb/influxdb3_wal/src/serialize.rs

Lines 32 to 33 in b2f6c1a

    
           /// The first bytes written into a wal file to identify it and its version. 
        
           const FILE_TYPE_IDENTIFIER: &[u8] = b"idb3.001";

This change adds a startup time counter to the output when starting up a server. The main purpose of this is to verify whether the impact of changes actually speeds up the loading of the server.

This commit does a few important things to speedup startup times: 1. We avoid changing an Arc<str> to a String with the series key as the From<String> impl will call with_column which will then turn it into an Arc<str> again. Instead we can just call `with_column` directly and pass in the iterator without also collecting into a Vec<String> 2. We switch to using bitcode as the serialization format for the WAL. This significantly reduces startup time as this format is faster to use instead of JSON, which was eating up massive amounts of time. Part of this change involves not using the tag feature of serde as it's currently not supported by bincode 3. We also parallelize reading and deserializing the WAL files before we then apply them in order. This reduces time waiting on IO and we eagerly evaluate each spawned task in order as much as possible. This gives us about a 189% speedup over what we were doing before. Closes #25534

mgattozzi · 2024-12-12T16:21:08Z

I'll merge once CI passes again as all I needed to do is remove the debug option in quick-release.

pauldix

One small comment, but not a blocker

pauldix · 2024-12-12T16:37:35Z

influxdb3_server/src/lib.rs

@@ -206,6 +212,9 @@ where
    let hybrid_make_service = hybrid(rest_service, grpc_service);

    let addr = AddrIncoming::from_listener(server.listener)?;
+    let timer_end = Instant::now();
+    let startup_time = timer_end.duration_since(startup_timer);
+    info!("Server Startup Time: {}ms", startup_time.as_millis());


maybe better to output this using duration's formatting? I think it'll do things like 1m32s...

@pauldix it does not have a formatting available to it by default. It does not impl Display. This is why you need to specify how you want it, and mostly it seems to do fractional (e.g. 2.7 seconds) rather than absolute (e.g. 2s 700ms) for the various methods. I figured ms was the most accurate, but I'm willing to do this method instead to get something like 2.7s or 800.5s

mgattozzi requested review from pauldix, hiltontj, jacksonrnewhouse and praveen-influx December 11, 2024 18:42

mgattozzi force-pushed the mgattozzi/startup-times branch from 5e83ae9 to a32801b Compare December 11, 2024 18:49

mgattozzi changed the title ~~feat: Significantly increase startup times for WAL~~ feat: Significantly decrease startup times for WAL Dec 11, 2024

hiltontj approved these changes Dec 11, 2024

View reviewed changes

praveen-influx approved these changes Dec 12, 2024

View reviewed changes

mgattozzi added 2 commits December 12, 2024 11:19

feat: add startup time to logging output

f850525

This change adds a startup time counter to the output when starting up a server. The main purpose of this is to verify whether the impact of changes actually speeds up the loading of the server.

mgattozzi force-pushed the mgattozzi/startup-times branch from a32801b to c9f4286 Compare December 12, 2024 16:19

mgattozzi merged commit 9292a32 into main Dec 12, 2024
13 checks passed

mgattozzi deleted the mgattozzi/startup-times branch December 12, 2024 16:27

pauldix reviewed Dec 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Significantly decrease startup times for WAL #25643

feat: Significantly decrease startup times for WAL #25643

mgattozzi commented Dec 11, 2024 •

edited

Loading

hiltontj commented Dec 11, 2024

mgattozzi commented Dec 11, 2024

hiltontj commented Dec 11, 2024

hiltontj left a comment

hiltontj Dec 11, 2024

mgattozzi Dec 12, 2024

praveen-influx left a comment

pauldix commented Dec 12, 2024

mgattozzi commented Dec 12, 2024

mgattozzi commented Dec 12, 2024

mgattozzi commented Dec 12, 2024

pauldix left a comment

pauldix Dec 12, 2024

mgattozzi Dec 12, 2024 •

edited

Loading

feat: Significantly decrease startup times for WAL #25643

feat: Significantly decrease startup times for WAL #25643

Conversation

mgattozzi commented Dec 11, 2024 • edited Loading

hiltontj commented Dec 11, 2024

mgattozzi commented Dec 11, 2024

hiltontj commented Dec 11, 2024

hiltontj left a comment

Choose a reason for hiding this comment

hiltontj Dec 11, 2024

Choose a reason for hiding this comment

mgattozzi Dec 12, 2024

Choose a reason for hiding this comment

praveen-influx left a comment

Choose a reason for hiding this comment

pauldix commented Dec 12, 2024

mgattozzi commented Dec 12, 2024

mgattozzi commented Dec 12, 2024

mgattozzi commented Dec 12, 2024

pauldix left a comment

Choose a reason for hiding this comment

pauldix Dec 12, 2024

Choose a reason for hiding this comment

mgattozzi Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

mgattozzi commented Dec 11, 2024 •

edited

Loading

mgattozzi Dec 12, 2024 •

edited

Loading