I don't understand the 0.5 second rule. For the tests, we only need to ensure that at least one progress report is printed, right?

[...]

I still don't understand.

Let's look at the code:

   if (progress && thread->tid == 0)
   {
      ...
      if (last_report == thread_start || now - last_report >= 500000)
        doProgressReport(thread, &last, &last_report, now, thread_start);

For the testing, we just need to make sure that at least one progress report is always printed, if -P is used. Right?

Yep. That is the first condition above the last_report is set to thread_start meaning that there has been no report.

So where does the 0.5 second rule come in? Can't we just do "if (no progress reports were printed) { print progress report; }" at the end?

The second 0.5s condition is to print a closing report if some time significant time passed since the last one, but we do not want to print
a report at the same second.

  pgbench -T 5 -P 2

Would then print report at 2, 4 and 5. 0.5 ensures that we are not going to do "2 4.0[00] 4.0[01]" on -t whatever -P 2, which would look silly.

If you do not like it then the second condition can be removed, fine with me.

It also adds a small feature which is that there is always a final
progress when the run is completed, which can be useful when computing
progress statistics, otherwise some transactions could not be reported in
any progress at all.

Any transactions in the last 0.5 seconds might still not get reported in any progress reports.

Yep. I wanted a reasonable threshold, given that both -T and -P are in seconds anyway, so it probably could only happen with -t.

Indeed… but then throttling would not be tested:-) The point of the test
is to exercise all time-related options, including throttling with a
reasonable small value.

Ok. I don't think that's really worthwhile. If we add some code that only runs in testing, then we're not really testing the real thing. I wouldn't trust the test to tell much. Let's just leave out that magic environment variable thing, and try to get the rest of the patch finished.

If you remove the environment, then some checks need to be removed, because the 2 second run may be randomly shorten when there is nothing to do. If not, the test will fail underterminiscally, which is not acceptable. Hence the hack. I agree that it is not beautiful.

The more reasonable alternative could be to always last 2 seconds under -T 2, even if the execution can be shorten because there is nothing to do at all, i.e. remove the environment-based condition but keep the sleep.

--
Fabien.

Reply via email to