Support "current position" (position=null) writes, reads

[innerop]

Hi

Awesome that you made this.

I’d like to do a benchmark based on your work for fileAppend and wondering if I can supply nulls for offset in position in writeBuffer so it just appends the data at end of file.

Is that supported?

If not, I can take a stab at it, although my knowledge of C++ is very rusty.

Thanks for sharing!

[zbjornson]

Hey, thanks for the interest.

null can't be passed yet (see TODO here:

node-io-uring/iou.cc

Line 115 in e28d354

* 4 position int64 position in file [TODO null for current position]

). I keep meaning to ask @axboe if non-positional (current position) file read/writes are going to be possible in the future as io_uring doesn't support it right now. Until then, I think Node would have to use lseek(2) to get and update the file position, but that makes it a non-atomic operation.

[axboe]

That doesn't work so well for async interfaces, so no, there will not be an option to use 'current' position.

[idibidiart]

@zbjornson

For my test, there's no possibility of concurrent writes to the file, so not having it be atomic is fine. I'm thinking of getting current file size from fs.stat using that that as the position argument (current end of file) -- see any issues with that? (I'll isolate the time cost of fs.stat call)

Thanks once again, and excited to see how well it performs in append mode

[zbjornson]

@idibidiart yes, fs.stat would work if you're appending to the end of a file. Ultimately lseek(2) would probably be the correct way to do it (or just have Node/libuv maintain the current position internally).

Note that this module still requires a custom Linux kernel build. The required patch is now in the mainline kernel but hasn't been released yet. If you don't want to build your own kernel but can provide benchmarking code, I'm happy to run it for you.

[idibidiart]

@zbjornson having Node/libuv maintain the position in memory would imply statefulness between writes and if the connection is not persistent (e.g. http) the request (in a real life scenario) could go to any server (assuming a shared fs) so like you say lseek(2) should be used... Does that mean (for now and until node/libuv adopt uring) you could expose the lseek functionality in the bindings?

I've built Linux kernel for embedded systems in the past and it wasn't not bad at all. I'd like to benchmark this on both bare metal and an EC2 I3 instance. Having said that, please feel free to do an append mode benchmark if you're interested in that and don't wait on me since this is exclusively a curiosity driven activity on my end :) and not sure when I'll get to it, but I wanted to know that it is possible in the first place. Exposing lseek behavior in the bindings would be great (if it makes sense) but in the meantime I suppose fs.stat will do....

You may close this issue or keep it open, but I'll post here anyway if/when I have some results to share.

Thank you so much.

[zbjornson]

@idibidiart I'm happy to make the lseek(2) changes so current-position writes work. Can do tomorrow or Thursday probably.

If you're comfortable doing your own kernel build, by all means, go ahead :). No rush from my side, I wasn't planning to work much on this until another kernel release is coming up. (Before that, there's no point pulling it into libuv.)

[idibidiart]

@zbjornson

I've put together a demo (for work) that uses a chunk buffer (a chunk can be up to e.g. 32MB) where each chunk comes in 64 realtime-compressed slices over websocket, so I can send progress events back) then write it to fs via createWriteStream with default highWaterMark of 64KB. The realtime compression is via a C++ native module, using SoTA compression algorithm. Writing each chunk takes less time than getting all the compressed slices across the network, but to play it safe, I'm using createWriteStream to avoid invoking fs.write in none strictly serial manner. I should be able to use your writeBuffer once you have seek support or with fs.stat (although the latter will slow things down)

Here's a demo (sorry, the code belongs to employer but explanation above gives an idea of how it works, which is not novel by any chance, except maybe in the result:)

https://www.youtube.com/watch?v=dHOY1dnzViM

[idibidiart]

but I'd like to benchmark it first... :)

[zbjornson]

Node.js' fs.WriteStreams actually maintain their position internally and pass that to fs.write; they don't use current-position writes. https://github.com/nodejs/node/blob/03d43539f93be2c401f57d7eac87a5ae933ef905/lib/internal/fs/streams.js#L312 Using current-position writes wouldn't make fs.write safely append your buffers in sequence if you can't guarantee that you call fs.write serially.

You might be able to do something like require("fs").write = require("node-io-uring").writeBuffer to make WriteStreams use this module.

It's also safe to call writeBuffer multiple times without waiting for a prior one to complete. (I don't know why Node's fs.write claims to be unsafe for this purpose as long as you provide a file position and don't open the file with O_APPEND; pwrite(2) is thread-safe.)

So maybe something like this would work?

let position = 0;
client.onSlice = slice => {
  iou.writeBuffer(fd, slice, 0, slice.length, position, (err, bytesWritten) => {
    // Handle if err or bytesWritten< slice.length
    // Wait for all callbacks to be called --> done
  });
  position += slice.length;
};

You mentioned what sounds like multiple servers processing your data, but file descriptors and thus fd positions are local to each process, so that doesn't help your scenario, if I understand correctly; unless you're doing something fancy with sharing fds (and even then, I don't know if positions are shared).

---

(Side note: you might also benchmark using fs.writeSync instead of an fs.WriteStream. In the application I work on for work, that sped up overall performance nearly 2x. You still get some async work happening, as the kernel is able to read from the socket while the Node thread is blocked writing.)

[zbjornson]

I don't know why Node's fs.write claims to be unsafe for this purpose as long as you provide a file position and don't open the file with O_APPEND; pwrite(2) is thread-safe.

Found the answer: nodejs/node#18634. It indeed sounds safe if you're always on POSIX.

[idibidiart]

Using current-position writes wouldn't make fs.write safely append your buffers in sequence if you can't guarantee that you call fs.write serially.

My mind has trouble understanding the rationale behind lseek. What use cases are there for moving the current read/write position statefully instead of just specifying the position when writing/reading?

I realize that createWriteStream keeps track of the position on JS side and even though I read the current file size from disk before I start calling WriteableStream.write on each connection it's possible that I end up with concurrent write from another server (e.g. after a disconnection/reconnection with no sticky session) However, when the socket disconnects the WriteableStream is ended immediately and it takes longer for the connection to reestablish (2 seconds delay) on a different server than for the WriteableStream to be closed, unless the event loop is overwhelmed.

But I am hoping the underlying fs.write is threadsafe (either writes the data or doesn't and won't write if another process is writing to the file.) Thanks for confirming that.

Does that mean your bindings.writeBuffer is thread safe, too? I assume it is, in which case I'll try what you suggested with the require swap. But first I gotta test your hint that writeSync is faster than WriteableStream.write. I'm buffering anyway.

Wow. Much to learn. Thank you.

[idibidiart]

I'll post the results of moving to writeSync and then move on to trying bindings.writeBuffer and keeping track of position in JS.

I really do not see benefit after this dialog of adding lseek for append (at current position) kind of behavior, but for compatibility with node's fs api I guess it'll have to be done.

[idibidiart]

Looks like the way I was using WriteableStream.write by having it write every 32MB chunk (aka buffer) instead of every 0.5MB slice-of-a-chunk (aka slice) was going at maximum speed (for one upload at a time -- I've yet to benchmark many concurrent uploads) Having it write every 0.5MB slice in rapid succession is super slow in comparison. I can look at the code for WriteableStream.write and tell why but that's a waste of time since it does what it does and I'm using it in optimal way.

So ... there was no difference in moving to writeSync in my case, nor was there any difference in moving to write (done safely, with position supplied explicitly in write mode)

But now that I'm down to fs.write() I can easily plugin your writeBuffer and see how much I gain for a single upload as well as for many concurrent uploads.

Update:

The link to the patch that you have in the Readme points to an invalid commit now. I did see that axboe is working on io_uring still, so it maybe wise to wait for the release of the new kernel, although like I said building the Linux kernel is always fun =)

[innerop]

~ 2GB in 2 minutes over 4G-speed upload bandwidth, uploading 10 files at once.

https://www.youtube.com/watch?v=EnSt4yf1K0c

This is using fs.write with a 32MB buffer