Configuration file may not be reparsed when successive save operations are called on a low resolution filesystem

[Ryan J Ollos]

As discussed in comment:17:ticket:11069 and comment:13:ticket:11176, the configuration file might not be reparsed when successive save operations are invoked in a time interval less that is less than the resolution of the filesystem.

In #11069 we discussed how to fix the issue for the functional test environment. Changes will be proposed here that intend to fix the issue for a Trac instance running on a low resolution filesystem.

[Ryan J Ollos]

Proposed changes can be found in log:rjollos.git:t11329.

After putting the test cases in place, I encountered failures on EXT4 that could only be avoided by putting a sleep operation of up to 1e-2 between touch / save operations in the test case.

Waiting for the file modification time to change seems like the most general solution that should work on all platforms, regardless of the file timestamp resolution and how small an interval there is between touch or save operations.

[Remy Blank]

This solution could lead to multiple environment reloads when saving a configuration, under normal (non-test) operation.

Instead of trying to work around the low timestamp resolution of certain filesystems (and the per-platform differences), how about storing the last change time in a separate file (trac.ini.ts)?

[Remy Blank]

Replying to rblank:

This solution could lead to multiple environment reloads when saving a configuration, under normal (non-test) operation.

Ah, scratch that, I misunderstood something. But storing the timestamp as a file instead of using the mtime would still have the benefit of avoiding up to 1 second of delay in tests, and therefore avoid making them (even) slower.

[Ryan J Ollos]

Replying to rblank:

Ah, scratch that, I misunderstood something.

After you mentioned I was considering that this might be possible - in the while loop we wait for the file modification time to change in response to the equivalent of a touch -m command, and then save the file. If an option value was accessed in another thread in the time between the touch -m and the save, then I think the file would be needlessly reparsed. That time interval must be quite small however.

I might not be thinking about that correctly though since I still haven't sorted out the path by which parse_if_needed gets called when an option value is accessed, assuming that I'm even correct in assuming it works that way.

But storing the timestamp as a file instead of using the mtime would still have the benefit of avoiding up to 1 second of delay in tests, and therefore avoid making them (even) slower.

There are two relevant factors:

• the 1 second delay will only be seen on filesystems with a low resolution timestamp.
• the 1 second delay will only be seen when there are repeated save operations that would not each result in a change to the modification time of the configuration file.

Given those two factors, I'm not sure we'll ever see too much of a slowdown in the test execution time, and at least with the current set of tests we'll only see a few seconds slowdown. Either way we should certainly go for the most robust solution.

I'll investigate the timestamp in file solution. If I understand correctly, we could really just store anything that was unique and changed each time the config file changed, such as the fingerprint of the file, or a random hash, but the timestamp is simple and probably makes the most sense. Would there be any advantage to storing it in a database table (system?) rather than a file?

[Remy Blank]

Replying to rjollos:

Given those two factors, I'm not sure we'll ever see too much of a slowdown in the test execution time, and at least with the current set of tests we'll only see a few seconds slowdown.

Your explanation makes sense, yes. There's one easy way to find out: run the tests on an affected system, with and without the patch. If the difference is indeed only a few seconds, we shouldn't implement anything more complicated.

[Ryan J Ollos]

Here are the times for an OS with a low resolution timestamp filesystem (EXT3), based on n = 3 executions of each:

• Before: 203-211 seconds (with 13 test failures in the authorization tests, like the one shown in comment:17:ticket:11069)
• After: 253-257 seconds

On an OS with a high resolution timestamp filesystem (EXT4), the execution time is insignificantly affected by the change, and the execution time is 220-225 seconds. So I think the slowdown is probably closer to 25 seconds since the Before tests on EXT3 would take longer to execute if the failing tests ran to completion.

The slowdown is greater than I expected, but since most developers will work on a modern filesystem, I wouldn't expect it to affect development efforts much. Let me know if anyone thinks the solution will be problematic and I can investigate alternatives.

With the change in place, we should be able to update these hints:

• TracInstall
• TracEnvironment

since the configuration file should always be reparsed after a save operation, even on a platform with a low resolution timestamp. Regardless, it is probably better advice to use a platform with a high resolution timestamp.

I added an improvement to the functional tests, based on a hint from Jun in comment:54:ticket:5658: [31658dca/rjollos.git]. The full set of changes can be found in log:rjollos.git:t11329.2.