Edgewall Software
Modify

Opened 18 years ago

Closed 18 years ago

Last modified 7 years ago

#2485 closed defect (fixed)

trac-admin resync crash

Reported by: anonymous Owned by: Christian Boos
Priority: normal Milestone: 0.9.4
Component: version control Version: 0.9.3
Severity: major Keywords: svn pool resync
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

trac-admin resync crash for huge subversion repository since trac 0.9.1 (0.9.2 too). resync might have big memory leak. On trac 0.9 have too but it is small, so I can perform resync. Currently I need to perform resync, downgrade to trac 0.9, resync, upgrade to trac 0.9.2.

Attachments (2)

memorygraphwhileresync.png (7.6 KB ) - added by anonymous 18 years ago.
memory graph. all graph are in resyncing. at last, resync failed. (did not crash python at r2655)
revchanges.csv (5.2 KB ) - added by anonymous 18 years ago.
node_change summary. created trac 0.9

Download all attachments as: .zip

Change History (20)

comment:1 by Christian Boos, 18 years ago

Component: trac-adminversion control
Owner: changed from daniel to Christian Boos

Please try out the latest from source:0.9-stable

I believe you need r2625.

comment:2 by Christian Boos, 18 years ago

The link above should have been source:branches/0.9-stable (we should preview non existent path in source links as missing, btw).

Also, what makes you believe it's a memory leak? Do you have a backtrace?

r2625 actually introduced additional memory requirements, so I'm a bit worried here…

by anonymous, 18 years ago

Attachment: memorygraphwhileresync.png added

memory graph. all graph are in resyncing. at last, resync failed. (did not crash python at r2655)

comment:3 by anonymous, 18 years ago

Checkout from source:branches/0.9-stable@2655 and test it.

  • resync failed. (Python didn't crash.)
  • Two big memory usage points exist while resyncing (since 0.9.1 to this revision). Second point is big damage to memory.
  • The machine which has 1.5GB main memory machine + 1.5GB pagefile are second big memory usage point will up to 1.8GB and continue resync. But at last, python used 2GB user program memory and die.

comment:4 by Christian Boos, 18 years ago

Milestone: 0.9.3
Status: newassigned

Can you try to use attachment:ticket:1271:trac-0.9stable-r2652.resync-ranges.patch and do the resync in smaller intervals?

I wonder if it's a leak, or simply one huge changeset involving the creation of too many fs roots.

Take care to enable debugging information before doing the resync, see TracLogging. You'll then be able to see if it crashes always for the same changeset (huge changeset case) or not (leak case).

by anonymous, 18 years ago

Attachment: revchanges.csv added

node_change summary. created trac 0.9

comment:5 by anonymous, 18 years ago

resync range patch did not helps me. It causes always same revisions that are huge changesets. However, there are huge changeset between first and second, but they use less memory like as other tiny/small or empty changesets.

I attached node-change summary file between rev.1-500 that created by trac 0.9. It was queried trac's sqlite db.

On trac 0.9.1 or later, first leak point is rev.425 (on screen, last debug message is "DEBUG: Caching node change in [422]"), second is rev.489 (on screen, last debug message is "DEBUG: Caching node change in [484]") and die.

According to my attached file, changeset of rev.471 is much bigger than rev.489, but rev.471 (also most biggest changeset rev.30 too) uses less memory for creating node-change table.

comment:6 by Christian Boos, 18 years ago

I see. Actually what is relevant is the number of edits:

425,F,E,1836
471,F,E,1445
489,F,E,2797

More precisely, the number of different roots needed to retrieve the correct created path/rev of the old revision, for each file change.

0.9 resync works because there we don't attempt to retrieve this information.

OTOH, even if we ultimately succeed in resyncing, I think Trac will still have a hard time when trying to render your changeset [489]

comment:7 by anonymous, 18 years ago

OTOH, even if we ultimately succeed in resyncing, I think Trac will still have a hard time when trying to render your changeset [489]

I think so. But it is natural for this repository. If I couldn't stand it, I'll modify Trac by myself (if trac's db could rebuilt…)

comment:8 by anonymous, 18 years ago

Milestone: 0.9.30.9.4

comment:9 by anonymous, 18 years ago

Version: 0.9.20.9.3

I upgrade to trac 0.9.2 to 0.9.3, trac browser said "need resync". So I resync, but Python crash (max application memory(2Gb) over). resync at downgraded trac 0.9 didn't help. No way to resync for trac 0.9.3, so I couldn't use trac anymore. How do I do?

comment:10 by Christian Boos, 18 years ago

Apply the following patch:

Index: svn_fs.py
===================================================================
--- svn_fs.py	(revision 2746)
+++ svn_fs.py	(working copy)
@@ -483,13 +483,11 @@
             else:
                 action = Changeset.EDIT
                 b_path, b_rev = change.base_path, change.base_rev
-                if revroots.has_key(b_rev):
-                    b_root = revroots[b_rev]
-                else:
-                    b_root = fs.revision_root(self.fs_ptr, b_rev, pool())
-                    revroots[b_rev] = b_root
-                change.base_path = fs.node_created_path(b_root, b_path, pool())
-                change.base_rev = fs.node_created_rev(b_root, b_path, pool())
+                try:
+                    change.base_path = fs.node_created_path(root, b_path, pool())
+                    change.base_rev = fs.node_created_rev(root, b_path, pool())
+                except (SystemError, core.SubversionException):
+                    pass
             kind = _kindmap[change.item_kind]
             path = path[len(self.scope) - 1:]
             base_path = _scoped_path(self.scope, change.base_path)

That's a quick workaround, however, not the correct fix. With that patch, you'll loose some interesting information, but the resync will succeed, at least.

comment:11 by anonymous, 18 years ago

Thank you, but above patch didn't work for resync. (crashed Python)

comment:12 by anonymous, 18 years ago

I did partially merged-back, this works resync. This modification have a problem. In Changeset View, "(previous)" link always point to previous revision. So I may modify changeset.py too.

--- svn_fs.py   (revision 89)
+++ svn_fs.py   (local)
@@ -463,13 +463,13 @@
         idx = 0
         copies, deletions = {}, {}
         changes = []
-        revroots = {}
         for path, change in editor.changes.items():
             if not self.authz.has_permission(path):
                 # FIXME: what about base_path?
                 continue
             if not (path+'/').startswith(self.scope[1:]):
                 continue
+            base_path = _scoped_path(self.scope, change.base_path)
             action = ''
             if not change.path and change.base_path:
                 action = Changeset.DELETE
@@ -482,17 +482,8 @@
                     action = Changeset.ADD
             else:
                 action = Changeset.EDIT
-                b_path, b_rev = change.base_path, change.base_rev
-                if revroots.has_key(b_rev):
-                    b_root = revroots[b_rev]
-                else:
-                    b_root = fs.revision_root(self.fs_ptr, b_rev, pool())
-                    revroots[b_rev] = b_root
-                change.base_path = fs.node_created_path(b_root, b_path, pool())
-                change.base_rev = fs.node_created_rev(b_root, b_path, pool())
             kind = _kindmap[change.item_kind]
             path = path[len(self.scope) - 1:]
-            base_path = _scoped_path(self.scope, change.base_path)
             changes.append([path, kind, action, base_path, change.base_rev])
             idx += 1

No fs.node_created_path and fs.node_created_rev will resync done. Illegal pool usage?

comment:13 by Christian Boos, 18 years ago

Not illegal, but maybe not optimal either… Please try out the following patch:

--- svn_fs.py	(revision 2746)
+++ svn_fs.py	(working copy)
@@ -455,6 +455,7 @@
 
     def get_changes(self):
         pool = Pool(self.pool)
+        tmp = Pool(pool)
         root = fs.revision_root(self.fs_ptr, self.rev, pool())
         editor = repos.RevisionChangeCollector(self.fs_ptr, self.rev, pool())
         e_ptr, e_baton = delta.make_editor(editor, pool())
@@ -465,6 +466,7 @@
         changes = []
         revroots = {}
         for path, change in editor.changes.items():
+            tmp.clear()
             if not self.authz.has_permission(path):
                 # FIXME: what about base_path?
                 continue
@@ -488,8 +490,8 @@
                 else:
                     b_root = fs.revision_root(self.fs_ptr, b_rev, pool())
                     revroots[b_rev] = b_root
-                change.base_path = fs.node_created_path(b_root, b_path, pool())
-                change.base_rev = fs.node_created_rev(b_root, b_path, pool())
+                change.base_path = fs.node_created_path(b_root, b_path, tmp())
+                change.base_rev = fs.node_created_rev(b_root, b_path, tmp())
             kind = _kindmap[change.item_kind]
             path = path[len(self.scope) - 1:]
             base_path = _scoped_path(self.scope, change.base_path)

comment:14 by anonymous, 18 years ago

It works! Thank you.

comment:15 by Christian Boos, 18 years ago

Keywords: svn pool added

Well, thank you for having pointed me in the right direction: I was sure the heavy memory usage came from the fs.revision_root calls, never thought that it could be the fs.node_created_* ones!

What's the memory usage pattern now, still >1G ?

comment:16 by anonymous, 18 years ago

No, little. resync used approx. 170Mb(working set & its peak), 165Mb(private bytes), 330Mb(virtual bytes & its peak).

comment:17 by Christian Boos, 18 years ago

Resolution: fixed
Status: assignedclosed

I committed the fix in [2756] and [2757].

comment:18 by Christian Boos, 18 years ago

Keywords: resync added

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.