Modify

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#10871 closed defect (fixed)

AccountGuard destroys trac.ini

Reported by: anonymous Owned by: hasienda
Priority: normal Component: AccountManagerPlugin
Severity: blocker Keywords: corruption
Cc: rjollos Trac Release: 0.11

Description

in the AccountGuard init the trac.ini config file is saved on each and every request. If you have multiple requests or fast reloads this leads to corrupt trac.ini and slows down the entire system drastically.

And all that just to avoid conflicting config settings?

IMHO I think it is better to throw a fatal error than this

Attachments (0)

Change History (11)

comment:1 Changed 2 years ago by anonymous

this can be found in 0.11 as well as in trunk

comment:2 Changed 2 years ago by anonymous

  • Cc me@… added; anonymous removed

comment:3 in reply to: ↑ description Changed 2 years ago by hasienda

  • Keywords corruption added
  • Trac Release changed from 1.0 to 0.11

Replying to anonymous:

in the AccountGuard init the trac.ini config file is saved on each and every request. If you have multiple requests or fast reloads this leads to corrupt trac.ini and slows down the entire system drastically.

Since yesterday I could reproduce this in my development environment too, and I've been already on the way to push a fix. No sure, why it did not surface even earlier. But thanks for taking your time to report, a nice confirmation to my own observations.

Minor nit-picking: Not "each and every request", because <class>.__init__ is executed just once per Trac environment reload. This makes a huge difference, still the issue is severe enough with re-write per reload (spawning yet another reload).

And all that just to avoid conflicting config settings? IMHO I think it is better to throw a fatal error than this

Agree. Even the fatal error wouldn't be justified IMHO.

comment:4 Changed 2 years ago by hasienda

(In [12609]) AccountManagerPlugin: Prevent trac.ini corruption by AccountGuard.__init__, refs #8930 and #10871.

Mostly disabled the configuration re-write introduced in [12468], because I did suffer trac.ini truncation by fast save-reload-cycles repeatedly, and this has been confirmed by the anonymous reporter of #10871 lately too.

Btw, I do plan to provide a better way to sanitize configurations within the current re-work of the configuration admin panel, see #8930.

comment:5 follow-up: Changed 2 years ago by anonymous

  • Cc anonymous added; me@… removed

that was fast thanks! :)

comment:6 Changed 2 years ago by hasienda

  • Resolution set to fixed
  • Status changed from new to closed

(In [12610]) AccountManagerPlugin: Publish maintenance release 0.4.3, closes #8927, #10681, #10765 and #10871.

This is another update for current stable acct_mgr-0.4 to immediatly push the fix against trac.ini corruption and other recent corrections.

Note though that an unnecessary msgid change from [12490] has been excluded and - as exception to the rule - there is a solution rated as 'feature' too.

comment:7 Changed 2 years ago by hasienda

(In [12611]) AccountManagerPlugin: Sync 0.11 branch to current stable release acct_mgr-0.4.3, refs #10871.

comment:8 in reply to: ↑ 5 Changed 2 years ago by hasienda

  • Cc rjollos added; anonymous removed

Replying to anonymous:

that was fast thanks! :)

You're welcome, but after thinking a bit more, this turned my plans to further hold-back some other fixes too. This is my consequence of claiming a release "stable", as I do care. Thanks again for pushing the issue.

comment:9 Changed 2 years ago by hasienda

(In [12612]) AccountManagerPlugin: Some small corrections for packaging of acct_mgr-0.4.3, refs #10871.

Thanks to Jun Omae for noticing about it.

I took the chance to finally sort entries in checksum files, since their order looks semi-random and caused a lot of noise in diffs on each update.

comment:10 Changed 2 years ago by hasienda

(In [12613]) AccountManagerPlugin: Sync 0.11 branch to updated release acct_mgr-0.4.3 again, refs #10871.

comment:11 Changed 2 years ago by hasienda

(In [12626]) AccountManagerPlugin: Wizard pages move into config admin panel, refs #8930 and #10871.

This new admin web-UI should become the single best choice to deal with the configuration. Even experts will value the checks for coherent configuration, that come bundled with the new wizard-style configuration admin panel. If you deal with more than an initial configuration, sooner or later you will get surprised (or doomed) by some dependency or side-effect. Honestly, me too.

Settings are written back now, but saved to trac.ini only on last page. For now I consider it a valuable feature to be able to test and eventually revert settings by just reloading the previous configuration from file. This has to be tested and user reception will decide too, if it could become the more successful approach.

All other components, that altered the configuration before, do it only in memory now to effectively prevent file corruption by too fast trac.ini reload cycles - lessons learned from #10871.

Included some missed goodies and more learned from Jun Omae's early review to improve for i18n.

Add Comment

Modify Ticket

Action
as closed The owner will remain hasienda.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.