performancecopilot / pcp

Performance Co-Pilot
https://pcp.io
Other
974 stars 237 forks source link

libpcp: new timecontrol protocol version: pmTimeval -> __pmTimestamp #1380

Open natoscott opened 3 years ago

kmcdonell commented 2 years ago

Since pmtime <--> client connections are always local (can someone who knows confirm this, because I can't see a way of specifying a remote hostname in the pmtime port configuration options), and since we ship all of the pmtime, libpcp_qmc, libpcp_qed, pmchart, etc. pieces as part of PCP and no one else uses the time control APIs, I can't see any reason that would prevent me changing the libpcp_qmc and libpcp_qed APIs and the pmtime internals:

Can anyone see a problem with this?

kmcdonell commented 2 years ago

On reflection, I now think there is nothing to do here! The protocol packet appears to be a Qmc::Packet and therein the timestamps are struct timevals (not explicit 32-bit ints like in the PDU and archive cases) ... so this will just work in 2038 by coat-tailing on the assumption that a time_t will be 64-bit everywhere by then and so struct timeval uses will all be safe. We don't have nsec precision (compared to __pmTimestamp or struct timespec uses), but I think this is OK here.

natoscott commented 2 years ago

Yep, agreed - its safe for Y2038 but doesn't allow tools like pmchart to support nanosecond precision. Which is OK for now I think - we can add this at any time if we wish.

I have a to-do item over in #1561 to support high res timestamps in libpcp_gui which builds on top of this protocol - and I think there's other improvements warranted here in terms of the protocol (e.g. we should use a unix domain socket here, not an inet socket, as its localhost-only and inet ports have their own set of issues, like conflicting with other services, etc).

These are all part of a bigger change, so how about I take these all on in one hit? (feel free to assign this my way if we're in agreement there)

kmcdonell commented 2 years ago

Agreed, and #1561 is already assigned to your good self.