Modify

Opened 18 months ago

Closed 18 months ago

Last modified 18 months 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 18 months ago by anonymous

this can be found in 0.11 as well as in trunk

comment:2 Changed 18 months ago by anonymous

  • Cc me@… added

comment:3 in reply to: ↑ description Changed 18 months 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 18 months 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 18 months ago by anonymous

  • Cc me@… removed

that was fast thanks! :)

comment:6 Changed 18 months 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 18 months 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 18 months ago by hasienda

  • Cc rjollos added

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 18 months 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 18 months ago by hasienda

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

comment:11 Changed 18 months 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 .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from hasienda. Next status will be 'closed'.
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.