nats-io / nats.rs

Rust client for NATS, the cloud native messaging system.
Apache License 2.0
1.06k stars 168 forks source link

KV Store should warn / error if you try upload items bigger then 1MB. #1093

Closed TroyKomodo closed 7 months ago

TroyKomodo commented 1 year ago

Currently If you try upload blobs bigger than 1MB you do not get an error and sometimes the ack just never is received.

There is a tracing error emitted

{"timestamp":"2023-08-20T14:12:29.851830Z","level":"ERROR","fields":{"message":"error handling command Broken pipe (os error 32)","log.target":"async_nats","log.module_path":"async_nats","log.file":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/lib.rs","log.line":385},"target":"async_nats","filename":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/lib.rs","line_number":385}

Which shows a general failure

Followed by

{"timestamp":"2023-08-20T14:12:29.851956Z","level":"INFO","fields":{"message":"event: disconnected","log.target":"async_nats::options","log.module_path":"async_nats::options","log.file":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/options.rs","log.line":111},"target":"async_nats::options","filename":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/options.rs","line_number":111}
{"timestamp":"2023-08-20T14:12:29.853720Z","level":"INFO","fields":{"message":"event: connected","log.target":"async_nats::options","log.module_path":"async_nats::options","log.file":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/options.rs","log.line":111},"target":"async_nats::options","filename":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/options.rs","line_number":111}
{"timestamp":"2023-08-20T14:12:29.853766Z","level":"INFO","fields":{"message":"event: connected","log.target":"async_nats::options","log.module_path":"async_nats::options","log.file":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/options.rs","log.line":111},"target":"async_nats::options","filename":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/options.rs","line_number":111}
{"timestamp":"2023-08-20T14:12:29.853791Z","level":"DEBUG","fields":{"message":"detected !Connected -> Connected state change","log.target":"async_nats::jetstream::consumer::pull","log.module_path":"async_nats::jetstream::consumer::pull","log.file":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/jetstream/consumer/pull.rs","log.line":892},"target":"async_nats::jetstream::consumer::pull","filename":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/jetstream/consumer/pull.rs","line_number":892}
{"timestamp":"2023-08-20T14:12:29.853851Z","level":"DEBUG","fields":{"message":"JetStream request sent: b\"{}\"","log.target":"async_nats::jetstream::context","log.module_path":"async_nats::jetstream::context","log.file":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/jetstream/context.rs","log.line":822},"target":"async_nats::jetstream::context","filename":"/home/troy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-nats-0.31.0/src/jetstream/context.rs","line_number":822}

Any NATs operations that were issued would just silently fail and timeout upon reconnect.

Nats server logs show

[1] 2023/08/20 14:12:29.851525 [ERR] 172.24.0.1:11680 - cid:36924 - maximum payload exceeded: 1248483 vs 1048576

So I assume that what I thought was a <1MB file was actually bigger so I should use object store but perhaps returning an error or logging a warning when you actually invoke the put command.

Jarema commented 1 year ago

Hey, thanks for filling in the issue.

You can increase the max payload size (though values above few megabytes are not advised). It's part of the server config.

We will add a check for publish payload size. I need to think about it, as it can change between reconnects and I would prefer to avoid any locks in publish.

TroyKomodo commented 1 year ago

Why not just use some sort of atomic value then? @Jarema

Jarema commented 1 year ago

We're working on that. Atomic value is an easy solution to implement, but it means reading that atomic value on every single publish, which I don't like.

Jarema commented 7 months ago

This has been fixed in https://github.com/nats-io/nats.rs/pull/1211 and is already released.