Opened 3 months ago

Closed 3 months ago

#33982 closed enhancement (fixed)

Simplify and refactor BytesSyncLogger

Reported by: dcf Owned by: dcf
Priority: Medium Milestone:
Component: Circumvention/Snowflake Version:
Severity: Normal Keywords:
Cc: cohosh, phw, arlolra, dcf Actual Points:
Parent ID: Points:
Reviewer: cohosh Sponsor:

Child Tickets

Change History (4)

comment:1 Changed 3 months ago by dcf

Status: assignedneeds_review

comment:2 Changed 3 months ago by cohosh

Reviewer: cohosh
Status: needs_reviewmerge_ready

Looks good to me.

We actually ripped out bytes logging from the proxy here after doing some CPU profiling (see #33211:comment:5). The logging at the client is more responsible because it does I/O at regular time intervals and not inside the copy loop, but I'm not sure how necessary it is. I use it when testing locally. We could check to see whether client logging is going to ioutil.Discard and if it is, not bother with bytes logging?

comment:3 Changed 3 months ago by dcf

Thanks for the review. Merged in 73173cb6987dbf26fdb1036e4b7710c200f87141.

I don't think there's a need to avoid the byte logger. Here in the client it's O(1) log lines per time interval, whereas in the proxy it was O(number of messages sent and received), which is why it got expensive at high data rates. If the scrubber is the expensive part, we could disable log scrubbing entirely when logging to ioutil.Discard, rather than disabling some log lines.

comment:4 Changed 3 months ago by dcf

Resolution: fixed
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.