Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DRAFT] Rework flush #1070

Closed
wants to merge 5 commits into from
Closed

[DRAFT] Rework flush #1070

wants to merge 5 commits into from

Conversation

Jarema
Copy link
Member

@Jarema Jarema commented Aug 2, 2023

This PR removes the need for manual flushing almost entirely.

What it does, is instead of relying on a timer, it tracks the number of pending Commands to be sent.

If there are no pending Commands, it's a slow traffic scenario, and the client flushes the data immediately.
If there are pending Commands, it buffers the data until buffer size is reached or pending commands reach 0.

Unfortunately, tokio::sync::mpsc does not allow checking how many messages are pending in the channel, hence the use of AtomicU64.

This can definitely by improved by replacing channels by some other shared state or simply another mpsc equivalent.

This is a quick check how this solution would perform.

benchmarks:

     Running benches/core_nats.rs (/Users/tomaszpietrek/coding/nats.rs/target/release/deps/core_nats-e2dc178e257d65b0)
async-nats: publish throughput/32
                        time:   [58.578 µs 59.552 µs 60.809 µs]
                        thrpt:  [50.186 MiB/s 51.245 MiB/s 52.097 MiB/s]
                 change:
                        time:   [-2.7784% -0.7490% +1.3989%] (p = 0.50 > 0.05)
                        thrpt:  [-1.3796% +0.7546% +2.8578%]
                        No change in performance detected.
Found 8 outliers among 30 measurements (26.67%)
  2 (6.67%) low mild
  2 (6.67%) high mild
  4 (13.33%) high severe
async-nats: publish throughput/1024
                        time:   [106.01 µs 106.68 µs 107.70 µs]
                        thrpt:  [906.76 MiB/s 915.39 MiB/s 921.17 MiB/s]
                 change:
                        time:   [-1.8603% -0.8395% +0.1396%] (p = 0.11 > 0.05)
                        thrpt:  [-0.1394% +0.8466% +1.8955%]
                        No change in performance detected.
Found 3 outliers among 30 measurements (10.00%)
  1 (3.33%) high mild
  2 (6.67%) high severe
async-nats: publish throughput/8192
                        time:   [643.61 µs 644.71 µs 645.99 µs]
                        thrpt:  [1.1810 GiB/s 1.1834 GiB/s 1.1854 GiB/s]
                 change:
                        time:   [-1.1147% +1.0207% +4.0855%] (p = 0.61 > 0.05)
                        thrpt:  [-3.9251% -1.0104% +1.1273%]
                        No change in performance detected.
Found 2 outliers among 30 measurements (6.67%)
  2 (6.67%) high severe

async-nats: publish messages amount/32
                        time:   [58.722 µs 59.763 µs 61.029 µs]
                        thrpt:  [1.6386 Melem/s 1.6733 Melem/s 1.7029 Melem/s]
                 change:
                        time:   [-2.2599% -0.2063% +1.6200%] (p = 0.84 > 0.05)
                        thrpt:  [-1.5942% +0.2068% +2.3121%]
                        No change in performance detected.
Found 6 outliers among 30 measurements (20.00%)
  6 (20.00%) high severe
async-nats: publish messages amount/1024
                        time:   [105.53 µs 105.71 µs 105.89 µs]
                        thrpt:  [944.36 Kelem/s 946.01 Kelem/s 947.56 Kelem/s]
                 change:
                        time:   [-2.3850% -1.5024% -0.8399%] (p = 0.00 < 0.05)
                        thrpt:  [+0.8470% +1.5253% +2.4433%]
                        Change within noise threshold.
Found 2 outliers among 30 measurements (6.67%)
  1 (3.33%) high mild
  1 (3.33%) high severe
async-nats: publish messages amount/8192
                        time:   [640.07 µs 644.38 µs 650.15 µs]
                        thrpt:  [153.81 Kelem/s 155.19 Kelem/s 156.23 Kelem/s]
                 change:
                        time:   [-0.9210% +0.3691% +1.9662%] (p = 0.65 > 0.05)
                        thrpt:  [-1.9283% -0.3677% +0.9296%]
                        No change in performance detected.
Found 4 outliers among 30 measurements (13.33%)
  3 (10.00%) high mild
  1 (3.33%) high severe

Benchmarking subscribe amount/32: Collecting 30 samples in estimated 5.1334 s (14k iterations)thread 'tokio-runtime-worker' panicked at 'called `Result::unwrap()` on an `Err` value: PublishError(SendError { .. })', async-nats/benches/core_nats.rs:87:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'tokio-runtime-worker' panicked at 'called `Result::unwrap()` on an `Err` value: PublishError(SendError { .. })', async-nats/benches/core_nats.rs:87:38
subscribe amount/32     time:   [342.40 µs 344.99 µs 347.71 µs]
                        thrpt:  [287.59 Kelem/s 289.86 Kelem/s 292.05 Kelem/s]
                 change:
                        time:   [-2.0785% -1.0757% -0.1275%] (p = 0.05 < 0.05)
                        thrpt:  [+0.1277% +1.0874% +2.1227%]
                        Change within noise threshold.
subscribe amount/1024   time:   [332.11 µs 334.44 µs 337.29 µs]
                        thrpt:  [296.48 Kelem/s 299.00 Kelem/s 301.10 Kelem/s]
                 change:
                        time:   [-1.4484% +0.1681% +1.9526%] (p = 0.85 > 0.05)
                        thrpt:  [-1.9152% -0.1678% +1.4697%]
                        No change in performance detected.
Found 2 outliers among 30 measurements (6.67%)
  1 (3.33%) high mild
  1 (3.33%) high severe
subscribe amount/8192   time:   [1.8358 ms 1.9150 ms 2.0246 ms]
                        thrpt:  [49.392 Kelem/s 52.220 Kelem/s 54.473 Kelem/s]
                 change:
                        time:   [-6.1529% -0.3343% +5.1627%] (p = 0.92 > 0.05)
                        thrpt:  [-4.9092% +0.3355% +6.5563%]
                        No change in performance detected.
Found 5 outliers among 30 measurements (16.67%)
  2 (6.67%) high mild
  3 (10.00%) high severe

@Jarema
Copy link
Member Author

Jarema commented Aug 4, 2023

needs benchmarks for full Core NATS + JetStream before merging.

missing core NATS request bench: #1079

Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
Signed-off-by: Tomasz Pietrek <tomasz@nats.io>
@Jarema Jarema closed this in #1060 Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant