chore: remove tokio01::codec #3095

fanatid · 2020-07-17T21:06:58Z

Ref. #2945 (comment)

Signed-off-by: Kirill Fomichev <[email protected]>

src/sources/socket/udp.rs

Signed-off-by: Kirill Fomichev <[email protected]>

Cargo.toml

src/async_read.rs

src/internal_events/unix.rs

Signed-off-by: Kirill Fomichev <[email protected]>

ktff · 2020-07-19T17:49:01Z

Can you add your name, for all of the done files in here, to #2945.

fanatid · 2020-07-19T18:00:10Z

Added. It's can be hard to make changes while this not merged, but I think changing shutdown to future:0.3 should not be blocked and require a lot of rebases in future. I'll address all comments here tomorrow, so hope that we will able merge this quickly.

Signed-off-by: Kirill Fomichev <[email protected]>

src/sinks/util/tcp.rs

src/sinks/statsd.rs

src/sinks/util/tcp.rs

MOZGIII · 2020-07-21T09:57:36Z

src/sinks/util/tcp.rs

@@ -225,13 +241,20 @@ impl Sink for TcpSink {
                        emit!(TcpEventSent {
                            byte_size: line.len()
                        });
+                        let line = Bytes::copy_from_slice(&line);


This a conversion between two bytes crates in unfortunate. The extra copies are probably not that big of a hit to the performance, and it's temporary, so I guess it's ok. Just pointing it out.

Yeah. The bad part of such slow update that in the future we will need to find and rid all such conversions. Also new bytes have extra copying on converting between Bytes <=> BytesMut 😞

True, yet it seems kind of odd that Bytes were convertible to BytesMut before in the first place. The API seems to have been incorrect, thus we're probably using it incorrectly... We probably should do an audit of the code as part of old bytes removal - I bet there are a lot of places where we can avoid copying where we could just switch to Bytes to BytesMut (or do sth similar).

An interesting thing, will clippy catch extra operations with Bytes or not. I already started improving bytes from BytesMut => Bytes through freeze, but this pretty hard for my current Rust level. tokio-rs/bytes#418

Signed-off-by: Kirill Fomichev <[email protected]>

lukesteensen

There's a lot here, but it generally looks reasonable to me. Left a few notes on things that may or may not be important, but shouldn't block. If CI is passing, then I'm good to merge.

In the future, it would be helpful to break down changes like this into a couple of pull requests. Smaller diff and more limited scope makes them quicker to review, quicker to merge, fewer conflicts, etc.

lukesteensen · 2020-07-21T22:21:09Z

src/sinks/util/unix.rs

 }

-fn unix_healthcheck(path: PathBuf) -> Healthcheck {
-    // Lazy to avoid immediately connecting
-    let check = future::lazy(move || {


I think the reasoning for lazy is still valid here unless you've verified that connect doesn't bind to the port until polled.

Good note. I actually do not understand why new futures need lazy function, because new futures not executed while not pulled, for example: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=1903bb7388f8d2965ee127bf8db2675c

why new futures need lazy function,

I think it's because Rust can't yet pass bare async closures around, thus this future::lazy.

Here: https://docs.rs/futures-util/0.3.5/src/futures_util/future/lazy.rs.html#34-38, it's very similar to poll! macro: https://docs.rs/futures-util/0.3.5/src/futures_util/async_await/poll.rs.html#13-17

src/sinks/util/unix.rs

src/sinks/util/tcp.rs

src/sources/util/tcp.rs

lukesteensen · 2020-07-21T22:41:18Z

src/sources/syslog.rs

-            let socket = UdpSocket::bind(&addr).expect("failed to bind to udp listener socket");
-
+        async move {
+            let socket = UdpSocket::bind(&addr)


Same as above for lazy and bind.

lukesteensen · 2020-07-21T22:41:41Z

src/sources/socket/udp.rs

@@ -39,41 +42,46 @@ pub fn udp(
    let out = out.sink_map_err(|e| error!("error sending event: {:?}", e));

    Box::new(
-        future::lazy(move || {
-            let socket = UdpSocket::bind(&address).expect("failed to bind to udp listener socket");


lukesteensen · 2020-07-21T22:42:40Z

src/sources/statsd/mod.rs

-            let socket = UdpSocket::bind(&addr).expect("failed to bind to udp listener socket");
-
+        async move {
+            let socket = UdpSocket::bind(&addr)


Here too 😄

Signed-off-by: Kirill Fomichev <[email protected]>

* chore: remove tokio01::codec Signed-off-by: Kirill Fomichev <[email protected]> Signed-off-by: Jesse Szwedko <[email protected]>

* chore: remove tokio01::codec Signed-off-by: Kirill Fomichev <[email protected]> Signed-off-by: Brian Menges <[email protected]>

fanatid added 9 commits July 17, 2020 11:06

statsd: remove tokio01 codec

a4b0362

Signed-off-by: Kirill Fomichev <[email protected]>

tokio-openssl: update with deprecate

060105c

Signed-off-by: Kirill Fomichev <[email protected]>

test_util: remove old codec

4c2e0f4

Signed-off-by: Kirill Fomichev <[email protected]>

benches/files: update codec

124d25e

Signed-off-by: Kirill Fomichev <[email protected]>

tests/syslog: update codec

1c6d856

Signed-off-by: Kirill Fomichev <[email protected]>

tests/tcp: remove codec

2aa9e4f

Signed-off-by: Kirill Fomichev <[email protected]>

sinks/util/unix: remove codec from test

749dad7

Signed-off-by: Kirill Fomichev <[email protected]>

sinks/util/unix: remove codec

a1700c9

Signed-off-by: Kirill Fomichev <[email protected]>

sinks/util/tcp: remove codec

9ece824

Signed-off-by: Kirill Fomichev <[email protected]>

fanatid added the type: tech debt A code change that does not add user value. label Jul 17, 2020

fanatid self-assigned this Jul 17, 2020

fanatid added 10 commits July 18, 2020 01:45

sources/socket/mod: remove tokio-uds

db9ab6d

Signed-off-by: Kirill Fomichev <[email protected]>

sources/socket/unix: update codec

edc626f

Signed-off-by: Kirill Fomichev <[email protected]>

sources/util/tcp: update codec

3d9a874

Signed-off-by: Kirill Fomichev <[email protected]>

sources/syslog: fix new codec

b874167

Signed-off-by: Kirill Fomichev <[email protected]>

sources/syslog: update codec

9ec7935

Signed-off-by: Kirill Fomichev <[email protected]>

sources/socket/udp: update codec

7349085

Signed-off-by: Kirill Fomichev <[email protected]>

remove crate codec01

f916a55

Signed-off-by: Kirill Fomichev <[email protected]>

add tokio-util/udp to features

b709dc7

Signed-off-by: Kirill Fomichev <[email protected]>

sources/statsd: update codec

3917d95

Signed-off-by: Kirill Fomichev <[email protected]>

remove unused tokio01 features: udp, codec

61071b1

Signed-off-by: Kirill Fomichev <[email protected]>

fanatid commented Jul 18, 2020

View reviewed changes

src/sources/socket/udp.rs Show resolved Hide resolved

fanatid marked this pull request as ready for review July 18, 2020 19:00

fanatid requested review from lukesteensen and MOZGIII as code owners July 18, 2020 19:00

cargo-fmt

4e3bff0

Signed-off-by: Kirill Fomichev <[email protected]>

MOZGIII reviewed Jul 19, 2020

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

MOZGIII reviewed Jul 19, 2020

View reviewed changes

src/async_read.rs Outdated Show resolved Hide resolved

MOZGIII reviewed Jul 19, 2020

View reviewed changes

src/internal_events/unix.rs Show resolved Hide resolved

remove already enabled feature

880f7d6

Signed-off-by: Kirill Fomichev <[email protected]>

fanatid requested a review from ktff July 19, 2020 16:52

fanatid added sink: socket Anything `socket` sink related sink: statsd Anything `statsd` sink related source: socket Anything `socket` source related source: syslog Anything `syslog` source related source: vector Anything `vector` source related labels Jul 19, 2020

lukesteensen mentioned this pull request Jul 19, 2020

Remove dependency on tokio 0.1 #2945

Closed

47 tasks

Merge remote-tracking branch 'upstream/master' into tokio01-codec-remove

e1b6d79

fanatid mentioned this pull request Jul 20, 2020

Add max_length to UDP socket source #3117

Closed

fanatid added 3 commits July 20, 2020 16:58

fix udp socket source

3cf9fb5

Signed-off-by: Kirill Fomichev <[email protected]>

fix syslog test for unix socket

56d8122

Signed-off-by: Kirill Fomichev <[email protected]>

Merge remote-tracking branch 'upstream/master' into tokio01-codec-remove

fc8cd1a

MOZGIII reviewed Jul 21, 2020

View reviewed changes

src/sinks/util/tcp.rs Show resolved Hide resolved

MOZGIII reviewed Jul 21, 2020

View reviewed changes

src/sinks/statsd.rs Outdated Show resolved Hide resolved

MOZGIII reviewed Jul 21, 2020

View reviewed changes

src/sinks/util/tcp.rs Outdated Show resolved Hide resolved

MOZGIII reviewed Jul 21, 2020

View reviewed changes

fanatid added 2 commits July 21, 2020 13:00

remove not required delay_for in statsd

fc4c810

Signed-off-by: Kirill Fomichev <[email protected]>

add type annotations for long get_ref/get_mut chains

4e5c05c

Signed-off-by: Kirill Fomichev <[email protected]>

binarylogic assigned lukesteensen Jul 21, 2020

binarylogic added this to the 2020.07.19 - The Great Orientation milestone Jul 21, 2020

lukesteensen approved these changes Jul 21, 2020

View reviewed changes

fanatid added 3 commits July 22, 2020 10:56

add explicit type to sinks/util

9c57084

Signed-off-by: Kirill Fomichev <[email protected]>

Merge remote-tracking branch 'upstream/master' into tokio01-codec-remove

90a2cb3

Merge remote-tracking branch 'upstream/master' into tokio01-codec-remove

b4bd844

fanatid merged commit f0f58f1 into master Jul 23, 2020

fanatid deleted the tokio01-codec-remove branch July 23, 2020 10:50

jszwedko pushed a commit that referenced this pull request Jul 24, 2020

chore: remove tokio01::codec (#3095)

b8506bf

* chore: remove tokio01::codec Signed-off-by: Kirill Fomichev <[email protected]> Signed-off-by: Jesse Szwedko <[email protected]>

mengesb pushed a commit to jacobbraaten/vector that referenced this pull request Dec 9, 2020

chore: remove tokio01::codec (vectordotdev#3095)

d1ca370

* chore: remove tokio01::codec Signed-off-by: Kirill Fomichev <[email protected]> Signed-off-by: Brian Menges <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: remove tokio01::codec #3095

chore: remove tokio01::codec #3095

fanatid commented Jul 17, 2020 •

edited

Loading

ktff commented Jul 19, 2020

fanatid commented Jul 19, 2020

MOZGIII Jul 21, 2020 •

edited

Loading

fanatid Jul 21, 2020

MOZGIII Jul 21, 2020 •

edited

Loading

fanatid Jul 21, 2020

lukesteensen left a comment

lukesteensen Jul 21, 2020

fanatid Jul 22, 2020

MOZGIII Jul 22, 2020 •

edited

Loading

lukesteensen Jul 21, 2020

lukesteensen Jul 21, 2020

lukesteensen Jul 21, 2020

chore: remove tokio01::codec #3095

chore: remove tokio01::codec #3095

Conversation

fanatid commented Jul 17, 2020 • edited Loading

ktff commented Jul 19, 2020

fanatid commented Jul 19, 2020

MOZGIII Jul 21, 2020 • edited Loading

Choose a reason for hiding this comment

fanatid Jul 21, 2020

Choose a reason for hiding this comment

MOZGIII Jul 21, 2020 • edited Loading

Choose a reason for hiding this comment

fanatid Jul 21, 2020

Choose a reason for hiding this comment

lukesteensen left a comment

Choose a reason for hiding this comment

lukesteensen Jul 21, 2020

Choose a reason for hiding this comment

fanatid Jul 22, 2020

Choose a reason for hiding this comment

MOZGIII Jul 22, 2020 • edited Loading

Choose a reason for hiding this comment

lukesteensen Jul 21, 2020

Choose a reason for hiding this comment

lukesteensen Jul 21, 2020

Choose a reason for hiding this comment

lukesteensen Jul 21, 2020

Choose a reason for hiding this comment

fanatid commented Jul 17, 2020 •

edited

Loading

MOZGIII Jul 21, 2020 •

edited

Loading

MOZGIII Jul 21, 2020 •

edited

Loading

MOZGIII Jul 22, 2020 •

edited

Loading