hyrise / hyrise

Hyrise is a research in-memory database.
https://hpi.de/plattner/projects/hyrise.html
MIT License
808 stars 158 forks source link

Is MetaSystemUtilizationTable::_get_total_time broken? #2164

Open mrks opened 4 years ago

mrks commented 4 years ago
/**
  * Returns the time in ns since epoch.
*/
uint64_t MetaSystemUtilizationTable::_get_total_time() {
  auto time = std::chrono::steady_clock::now().time_since_epoch();
  return std::chrono::duration_cast<std::chrono::nanoseconds>(time).count();
}

I don't think steady_clock has a usable epoch, so what is the purpose of this and of the total_time field in meta_system_utilization? Are we by any chance relying on an implementation detail where steady_clock happens to overlap with the wall time of the process?

j-tr commented 4 years ago

the total_time is returned so that this can be used to calculate the percentage of time that has been spent on the cpu, in idle state, etc. between two requests on the meta table. so the intended use is to get a time interval from two measurements. I used steady_clock because the documentation states: "This clock is not related to wall clock time (for example, it can be time since last reboot), and is most suitable for measuring intervals.". time_since_epoch is just used to convert the time_point into something that we can store in a table and if we are looking only at intervals, the epoch start is arbitrary. we could potentially have an overlap with the walltime if the process is never idling and the epoch start is set to the moment the process started... but i think this would be very unusual. However, i don't yet understand why this could cause problems.

mrks commented 4 years ago

Maybe I am just misreading this. Two follow-up questions:

j-tr commented 4 years ago

you are right, the client could track the time between requests as well, but this might not be as exact as if we track it due to scheduling, (network) latency, etc. i have no strong opinion against not using system_clock if you say that adjusting clocks are usually no problem for us.

mrks commented 4 years ago

you are right, the client could track the time between requests as well, but this might not be as exact as if we track it due to scheduling, (network) latency, etc.

That is somewhat the case for all meta tables though.

i have no strong opinion against not using system_clock if you say that adjusting clocks are usually no problem for us.

Not sure if there is a performance difference, but if not, it might be more intuitive.