@generall @timvisee
Hi, I think I'm ready for review ! :)

Hey @kemkemG0, thanks for the PR!

Before proceeding with the merge, I have a couple of follow-up questions regarding the proposed solution:

• It seems like fs2 depends on libc: https://github.com/danburkert/fs2-rs?tab=readme-ov-file#platforms, does it mean that our MUSL builds won't work? If so, is it possible to make this dependency and overall disk check optional?

• I tested the latency of the check - on my system (local SSD, ext4) it takes about 10µs average, which is fine. Is there an estimation what should we expect on different systems? Especially network-mounted disks. It it might have impact on the overall performance, is there a way to make those checks less frequent?

I would take a look at https://github.com/al8n/fs4-rs

Hi @generall !

I took a look at fs4 and it seems great; it uses rustix and should be able to build with MUSL as well. I'll proceed with switching from fs2 to fs4. : )

While I'm not an expert on file systems or the statvfs syscall, I believe the increase in response time can be considered as the sum of one RTT (Round-Trip Time) + one disk lookup and I assume these statistics are typically cached in memory?

Also, quoting from a blog by tokio maintainer ryhl:

"A good rule of thumb is no more than 10 to 100 microseconds between each .await."

Your benchmark shows about 10µs, and it can be expected that responses on other systems will also come back within 100µs. Therefore, I believe that keeping it synchronous might be fine, but wrapping it with tokio::task::spawn_blocking might also be a good option.

ps. updated !!
Use fs2 -> fs4 and offload sync IO with tokio::task::spawn_blocking

As a means to reduce these checks, I thought about changing the WAL flush function so that checks are only performed when wal's flushing. This would definitely reduce the number of calls, but since the WAL is used as an external crate, it could not be modified. If it were to be changed, it would involve forking and modifying the flush function to include the checks, but I'm worried this might be too complex for what we want to achieve.

Please let me know your thoughts or if there are any other aspects we should consider.

As a means to reduce these checks, I thought about changing the WAL flush function so that checks are only performed when wal's flushing.

Do you assume that the main source of the failures if WAL creating new segments?

Do you assume that the main source of the failures if WAL creating new segments?

IIRC I've also seen OOD failures in RocksDB. But checking for at least WAL capacity probably prevents that from happening too.

Here is some benchmarks on network-attached disk (7500 IOPS)

I am uploading 500k points in batch of 10 points with the following:

docker run --rm -it qdrant/bfb:dev ./bfb -n 500000 -b 10 -d 128 -p 2 --uri 'http://10.0.0.2:6334' --skip-wait-index

Result across 3 runs with low dispersion in results:

dev takes: 27 second to upload
branch takes: 31 second for the same

which gives almost 15% slowdown with the check.

I am afraid that would be too much and we would need to find some better way to implement this

@generall @timvisee

I've been considering two possible scenarios:

1. The write to database itself crush.
2. After nearly exhausts the disk space, and then the OS issues a system call using the disk and crashes. In fact, when debugging with println!, the function unrelated to the write continues to work until just before it is called, but it doesn't print right after the call, suggesting it's not a panic but a crash.

which gives almost 15% slowdown with the check.

I am afraid that would be too much and we would need to find some better way to implement this

Additionally, I mentioned

I thought about changing the WAL flush function so that checks are only performed when wal's flushing.

This is because in the current my implementation, a statvfs(3) call is made with each update request, but by calling the statvfs syscall only when the wal is flushing, I believe it could be sped up by a factor of wal_size/request_size.

Looking at the above performance test, maybe we can make it less intensive:

• periodically query free disk space in a background job
• or query on something like 1% of the update operations

I would probably prefer the first to prevent having a big hit on tail latencies. It could be part of our current flush loop, which might actually make sense because it's likely for writes to happen at that time.

I imagine this as a semi-global atomic in which we store the last read amount of free disk space. The current logic in this PR will just read that, versus doing a syscall in all cases.

Once we hit the condition we can query on 100% of the update requests to immediately recover once disk space is available.

@generall Thoughts?

One alternative: make this check less frequent by using timeout and number of operations.

So for example, check the disk space only if it was more than one second or more than N operations was performed since the last check

but since the WAL is used as an external crate, it could not be modified.

Wal is a Qdrant crate, you could send a PR to it. https://www.github.com/qdrant/wal

@generall @timvisee
To summarize:

1. Integrate a disk usage check as part of the flush loop to regularly monitor disk usage.
2. Also, Perform the disk usage check every N iterations.
3. Save the checked usage by mutating a shared variable.
4. When updating or inserting into the database, refer to this variable.

I will try proceeding with the implementation based on this plan :)

@generall
However, I feel that 1 might be sufficient, but why is 2 necessary?

@general @timvisee

I implemented DiskUsageWatcher , could you check it please?

BenchMark: 125MBps, 1100IOPS
docker run --rm -it --network host qdrant/bfb:dev ./bfb -n 500000 -b 10 -d 128 -p 2 --uri 'http://127.0.0.1:6334' --skip-wait-index

• this branch:
  • 65, 71, 64, 60, 64, 63 sec
• dev:
  • 64, 65, 63, 62, 65, 68 sec

There is some variation, but they appear to be within the same range.

However, I feel that 1 might be sufficient, but why is 2 necessary?

If the upsertion rate in terms of bytes is greater than the WAL size per flush interval, then it might fall through this check.

What are the above numbers? Do the represent the number of seconds for consecutive runs?

@timvisee
I see, then I wonder what the N should be, since every request can different size of insertion and can't decided fixed N :(

What are the above numbers? Do the represent the number of seconds for consecutive runs?
Yeah, they're the benchmarks on my end! I would like you to double check the benchmark as well to confirm if this branch's not that slow.

Another idea that might be realized by extending this implementation is to vary the frequency of updates based on free disk space.

For example, as far as I remember, the limit per request is 30MB, so how about an implementation where if there is more than 50MB(maybe 100MB is safer???) remaining, updates are made only during the flush loop, and if it's below 50MB, updates are made every time?

It may sounds heuristic, but the implementation shouldn't be too complex, and the accuracy is better.

Good question. The request size can be measured to determine this, but that may be over-engineering it.

Maybe we can pick N=1000 for now to refresh every 1000 checks. A thousand vectors with dimensionality 1500 are 6 megabytes. I'd suggest implementing that first. We can fine tune this later.

@timvisee

It sounds nice, updated with N = 1000 :)

I commented almost simultaneously as you, and probably it wasn't visible to you, so I would also like you to check the previous idea, thanks!

Hey @kemkemG0, we are going to merge it as-is now. Will also apply some minor updates later.

I like you idea about changing the check interval based on free disk size, I think we are going to follow it