Opened 5 weeks ago

Closed 4 weeks ago

#23552 closed defect (implemented)

We don't need to log our scheduler type so often

Reported by: pastly Owned by: pastly
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-sched
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

This is silly.

Sep 16 20:54:07.473 [notice] Tor 0.3.2.0-alpha-dev (git-4519b7b469635686) running on Linux with Libevent 2.1.8-stable, OpenSSL 1.0.2l, Zlib 1.2.11, Liblzma 5.2.3, and Libzstd N/A.
Sep 16 20:54:07.473 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Sep 16 20:54:07.473 [notice] This version is not a stable Tor release. Expect more bugs than usual.
Sep 16 20:54:07.473 [notice] Read configuration file "/home/matt/src/tor/torrc".
Sep 16 20:54:07.475 [warn] Path for DataDirectory (data) is relative and will resolve to /home/matt/src/tor/data. Is this what you wanted?
Sep 16 20:54:07.475 [warn] Path for HiddenServiceDir (data/hs1) is relative and will resolve to /home/matt/src/tor/data/hs1. Is this what you wanted?
Sep 16 20:54:07.475 [warn] ControlPort is open, but no authentication method has been configured.  This means that any program on your computer can reconfigure your Tor.  That's bad!  You should upgrade your Tor controller as soon as possible.
Sep 16 20:54:07.476 [notice] Scheduler type KIST has been enabled.
Sep 16 20:54:07.476 [notice] Opening Socks listener on 127.0.0.1:23456
Sep 16 20:54:07.476 [notice] Opening Control listener on 127.0.0.1:12345
Sep 16 20:54:07.476 [warn] Your log may contain sensitive information - you disabled SafeLogging. Don't log unless it serves an important reason. Overwrite the log afterwards.
Sep 16 20:54:07.477 [notice] Scheduler type KIST has been enabled.
Sep 16 20:54:07.680 [notice] Bootstrapped 0%: Starting
Sep 16 20:54:07.833 [notice] Scheduler type KIST has been enabled.
Sep 16 20:54:08.009 [notice] Starting with guard context "default"
Sep 16 20:54:08.014 [notice] Bootstrapped 80%: Connecting to the Tor network
Sep 16 20:54:08.757 [notice] Bootstrapped 85%: Finishing handshake with first hop
Sep 16 20:54:11.319 [notice] Bootstrapped 90%: Establishing a Tor circuit
Sep 16 20:54:13.575 [notice] Tor has successfully opened a circuit. Looks like client functionality is working.
Sep 16 20:54:13.575 [notice] Bootstrapped 100%: Done
Sep 16 20:54:15.049 [notice] Scheduler type KIST has been enabled.
Sep 16 22:23:16.435 [notice] Scheduler type KIST has been enabled.
Sep 17 00:18:14.736 [notice] Scheduler type KIST has been enabled.
Sep 17 01:32:13.524 [notice] Scheduler type KIST has been enabled.
Sep 17 02:54:08.478 [notice] Heartbeat: Tor's uptime is 5:59 hours, with 15 circuits open. I've sent 1.70 MB and received 3.21 MB.
Sep 17 03:20:14.427 [notice] Scheduler type KIST has been enabled.
Sep 17 05:00:11.271 [notice] Scheduler type KIST has been enabled.
Sep 17 06:46:11.481 [notice] Scheduler type KIST has been enabled.
Sep 17 08:06:14.916 [notice] Scheduler type KIST has been enabled.
Sep 17 08:54:08.480 [notice] Heartbeat: Tor's uptime is 11:59 hours, with 15 circuits open. I've sent 2.90 MB and received 6.01 MB.
Sep 17 09:18:12.910 [notice] Scheduler type KIST has been enabled.
Sep 17 11:15:13.930 [notice] Scheduler type KIST has been enabled.
Sep 17 12:21:16.363 [notice] Scheduler type KIST has been enabled.

I think we just want to log once on startup, then log only if it changes.

Child Tickets

Change History (10)

comment:1 Changed 5 weeks ago by pastly

Owner: set to dgoulet
Status: newassigned

comment:2 Changed 5 weeks ago by dgoulet

Status: assignedneeds_review

See branch bug23552_032_01.

comment:3 Changed 4 weeks ago by nickm

Status: needs_reviewneeds_revision

Two issues:

  • There are significant merge conflicts on master, wrt the code movement.
  • I'm not sure it's okay to use "-1" to mean "no type" unless we explicitly make "old_type" an int.

comment:4 Changed 4 weeks ago by pastly

Owner: changed from dgoulet to pastly
Status: needs_revisionassigned

comment:5 Changed 4 weeks ago by pastly

Status: assignedneeds_review

bug23552_032_02 on https://github.com/pastly/public-tor.git

Includes:

  • #23581 commit
  • A manual rebase onto the latest master to resolve code movement
  • SCHEDULER_NONE in the enum

comment:6 Changed 4 weeks ago by dgoulet

Seem this comment is wrong or misplaced:

+/** Scheduler type, we build an ordered list with those values from the
+ * parsed strings in Schedulers. The reason to do such a thing is so we can
+ * quickly and without parsing strings select the scheduler at anytime. */
+typedef enum {

I'm a bit worried here with the use of SCHEDULER_NONE = 0, not that it is logically wrong but because it is the "default type" that any scheduler object will get with a static initialization or tor_malloc_zero() that is 0. It just seems semantically weird to have this option... This is why in the first place, I started the first type at 1 so if you forget to set the type, some validation should catch it (well validation we don't have but you get the concept :P).

Anyway, no strong opinion if nickm is fine with this approach. Else, we can just make old_scheduler_type a int, set it to -1 and we are for sure certain that it can't be a scheduler type never ever.

comment:7 Changed 4 weeks ago by dgoulet

Status: needs_reviewneeds_revision

comment:8 Changed 4 weeks ago by pastly

Status: needs_revisionmerge_ready

Made SCHEDULER_NONE == -1. It doesn't look like old_scheduler_type needs to be an int.

bug23552_032_03 on https://github.com/pastly/public-tor.git

comment:9 Changed 4 weeks ago by dgoulet

lgtm;

comment:10 Changed 4 weeks ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merged; removed the now-unused chosen_scheduler_type local variable to fix a warning.

Note: See TracTickets for help on using tickets.