Defect #6579

Tree hierachy being currupted on multiple submissions of an issue

Added by Bruno Medeiros almost 8 years ago. Updated over 3 years ago.

Status:ClosedStart date:2010-10-05
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

100%

Category:Issues
Target version:3.0.0
Resolution:Fixed Affected version:1.0.2

Description

Frequently some user can accidentally press the 'Create' button twice (or more times) when creating a new subtask. In this case, not only the issue will be created a lot of times, but the issue tree will be corrupted.

I guess the insertion is being done without transaction isolation, and the lft and rgt fields are being calculated in the wrong conditions.

Although the bug reproduction depends on certain conditions, it's easy to reproduce it:
1. Press 'Add' to add some subtask to a existing issue
2. Fill some data in the new subtask
3. Press the 'Create' button a lot of times quickly, without wait the submit's end

After that, there will be some issues as grandson issues (not children issues as expected), but the parent task will be the right one.

I created a example of this problem on the redmine demo, at http://demo.redmine.org/issues/33583
All subtasks 'dd' in this example should be child of parent, but it's not this way.

I'm marking this as High priority because it corrupts the database without an easy fix.


Related issues

Related to Redmine - Defect #7666: Deleting multiple subtasks brokes nested set hierarchy Closed 2011-02-20
Related to Redmine - Defect #8537: Error in copying projects Closed 2011-06-06
Related to Redmine - Defect #6309: missing "not_a_valid_parent" from all the locale files? Closed 2010-09-06
Related to Redmine - Defect #14335: Issue.rebuild! "lft" and "rgt" are 1 and 2 Closed
Related to Redmine - Feature #18860: Replace awesome_nested_set gem with a custom implementati... Closed
Related to Redmine - Defect #15581: Another broken display of subtasks on parent issues Closed
Related to Redmine - Defect #19040: Potential DB deadlocks on concurrent issue creation Closed
Related to Redmine - Defect #8143: Problem to update subtask Closed 2011-04-13
Related to Redmine - Defect #19344: MySQL 5.6: IssueNestedSetConcurrencyTest#test_concurrency... New
Duplicated by Redmine - Defect #11772: Multiple parent task - maybe bug? Closed
Duplicated by Redmine - Defect #6143: Subtask creation form submission race? Closed 2010-08-16
Duplicated by Redmine - Defect #17430: Broken Subtask Tree Closed
Duplicated by Redmine - Defect #11678: MySQL locks and Redmine collapsing Closed

Associated revisions

Revision 12925
Added by Toshi MARUYAMA over 4 years ago

prevent issue tree hierarchy is broken in race conditions (#6579)

awesome_nested_set 2.1.6 uses lock.
Issue model uses as same way.

Revision 12926
Added by Toshi MARUYAMA over 4 years ago

revert r12925 of 2.5-stable

I mistook to commit in stable branch.

Revision 12927
Added by Toshi MARUYAMA over 4 years ago

prevent issue tree hierarchy is broken in race conditions (#6579)

awesome_nested_set 2.1.6 uses lock.
Issue model uses as same way.

Revision 13008
Added by Toshi MARUYAMA over 4 years ago

use bundled awesome_nested_set (#6579)

Revision 13009
Added by Toshi MARUYAMA over 4 years ago

fix always new lft and rgt are lft = 1, rgt = 2 (#6579)

Revision 13010
Added by Toshi MARUYAMA over 4 years ago

awesome_nested_set: split highest rgt reader method (#6579)

Revision 13011
Added by Toshi MARUYAMA over 4 years ago

fix race condition of highest rgt at Issue#update_nested_set_attributes_on_parent_change (#6579)

Revision 13014
Added by Toshi MARUYAMA over 4 years ago

fix awesome_nested_set test failure (#6579)

Revision 13143
Added by Toshi MARUYAMA about 4 years ago

awesome_nested_set: import git 2-1-stable branch revision 606847769 (#6579)

https://github.com/collectiveidea/awesome_nested_set/commit/606847769

Revision 13144
Added by Toshi MARUYAMA about 4 years ago

awesome_nested_set: import git 2-1-stable branch revision 3d5ac746542b564 (#6579)

https://github.com/collectiveidea/awesome_nested_set/commit/3d5ac746542b564

Revision 13145
Added by Toshi MARUYAMA about 4 years ago

awesome_nested_set: import git 2-1-stable branch revision 8eaab19868f326 (#6579)

https://github.com/collectiveidea/awesome_nested_set/commit/8eaab19868f326

Revision 13146
Added by Toshi MARUYAMA about 4 years ago

awesome_nested_set: add lib/plugins/awesome_nested_set/init.rb (#6579)

Revision 13147
Added by Toshi MARUYAMA about 4 years ago

remove awesome_nested_set patch from config/initializers/10-patches.rb (#6579)

Revision 13148
Added by Toshi MARUYAMA about 4 years ago

awesome_nested_set: not use cache for max rgt (#6579)

Revision 13152
Added by Toshi MARUYAMA about 4 years ago

Gemfile: upgrade sqlserver tiny_tds to 0.6.2 (#6579)

Revision 13153
Added by Jean-Philippe Lang about 4 years ago

Fixed "column specified more than once" error with SQLServer (#6579).

Revision 13345
Added by Toshi MARUYAMA almost 4 years ago

remove unneeded setting self[left_column_name] and self[right_column_name] from Issue#update_nested_set_attributes_on_parent_change (#6579)

Revision 14028
Added by Jean-Philippe Lang over 3 years ago

Adds a test for #6579.

History

#1 Updated by Bruno Medeiros almost 8 years ago

One more info:
The problem is even worse, because after one wrong submission, all new subtasks will become child of the last child, avoiding the creation of a subtask of the parent for ever.

#2 Updated by Mischa The Evil almost 8 years ago

As visible on the demo I've been able to reproduce the described behaviour there.
I don't know though if it's something that should be fixed inside Redmine. This seems more of an issue that might affect more Rails-based apps. Though I might be wrong here... 8-)

#3 Updated by Bruno Medeiros over 7 years ago

There's a simple fix to this, as did in #6826:

Disable 'Add' button after click!

#4 Updated by Andrew Vit over 7 years ago

I can have a look at this later today since I worked on #6826.

#5 Updated by Etienne Massip over 7 years ago

  • Category set to Issues
  • Target version set to Candidate for next minor release

#6 Updated by Bruno Medeiros over 7 years ago

Good to see it could be fixed soon. I can confirm this bug is still present on demo, and created another ticket there to demonstrate the problem:

http://demo.redmine.org/issues/2896

All descendants of 2896 should be children, but they are shown in different (wrong) levels.

As I said above, I believe the fix applied to #6826 can fix this too, without much work. Please post here if you need some help with this.

#7 Updated by Etienne Massip over 7 years ago

I wonder if this may be fixed with a transaction isolation explicit setting somewhere (if there is a transaction) ?

#8 Updated by Bruno Medeiros over 7 years ago

Don't know much about RoR, but at least in Java we're used to make each request have its own transaction, making this approach harder than avoid the double click.

The only downside I see on using the 'avoid double click' solution is if more than one user tries to create children for an issue at the exactly same time, which is very unlikely.

#9 Updated by Etienne Massip over 7 years ago

Well, there is no transaction and 2 successive DML (which explain #7667) : #save does the insert and #after_save => #update_nested_set_attributes does an update of the hierarchy.

Doing a single insert (that is, do not call #update_nested_set_attributes after save but before) can be buggy too.

Edit May, 11th : Actually, there is a transaction managed by RoR ActiveRecord.

#10 Updated by Alfredo Bonilla about 7 years ago

Hi, we have this problem in our installation too. Our "work around", re-create the task (duplicate) and re-do the parent relationship.
Here is part of the dump:

  app/models/issue.rb:442:in `soonest_start'
  /var/lib/gems/1.8/gems/ruby-ole-1.2.11.1/lib/ole/support.rb:40:in `send'
  /var/lib/gems/1.8/gems/ruby-ole-1.2.11.1/lib/ole/support.rb:40:in `to_proc'
  app/models/issue.rb:442:in `soonest_start'
  app/models/issue.rb:272:in `validate'
  app/models/issue.rb:498:in `save_issue_with_child_records'
  app/models/issue.rb:487:in `save_issue_with_child_records'

#11 Updated by Vasili Pupkin about 7 years ago

Hi, we have this problem too. My question is - can I manually set the lft/rgt values directly in DB instead of the copying the issues and recreating the hierarchy? What will be the proper values for lft/rgt and for what these columns are defined?

#12 Updated by Bruno Medeiros about 7 years ago

Yes, you can do it, but need to take care. you should do a backup first just in case.

lft and rgt tell redmine how it should build the tree. They hold the visit and unvisit index on a depth-first search on the tree. Example:

  • Parent (lft 1, rgt 10)
    • Child 1 (lft 2, rgt 3)
    • Child 2 (lft 4, rgt 9)
      • Grandson 1 (lft 5, rgt 6)
      • Grandson 2 (lft 7, rgt 8)

Parent is the first to be visited (lft=1), and the last to be unvisited (rgt=10), and so on... These should be calculated for all tickets with the same root. The root will have always lft=1 and rgt = #tickets * 2.

#13 Updated by Luis Serrano Aranda about 7 years ago

This also happens when copying a project

#14 Updated by Luis Serrano Aranda about 7 years ago

This it's a workaround...

#8537

This solves part of the problem, but pressing the back button and redo the operation

#16 Updated by Gary Shi over 6 years ago

Just met this problem in my environment (1.2.1). Surprised it's not fixed for such a long time.

If it's so hard to fix, is there any scripts that could fix or recalculate lft/rgt values after a curruption?

#17 Updated by Etienne Massip over 6 years ago

Gary Shi wrote:

(...) is there any scripts that could fix or recalculate lft/rgt values after a curruption?

Try to run

ruby script/runner -e production 'Project.rebuild!'

from your Redmine base directory.

Edit: I mean Issue.rebuild! but didn't try yet so I advise you to make a DB dump before.

#18 Updated by Matt Brown over 6 years ago

I found it possible to reset the parent/child relationships with:

UPDATE `redmine`.`projects` SET lft = 2 * id - 1, rgt = 2 * id;

Then adjust the subprojects relationship manually within the UI.

This will make each of the projects have no parents and no subordinates (as their left [bookend] and right [bookend] are just one number apart).

Knowing this is how the relationship is derived, should allow you to isolate problems more easily. You can actually remove sub-projects just by not having a lft and rgt number within a parent project's lft-rgt range.

I worked hard to migrate from 0.8.4 to 1.3.1 and had to deal directly with the DBs where this situation came into play.

#19 Updated by Eugene Hutorny about 6 years ago

Hit this issue with the following scenario:
Two users concurrently update two different issues and assign the same parent id to both of them.
Hierarchy of the parent task becomes broken and access to its children may cause process hanging or crashing.

SQL for detecting broken hierarchies:

select distinct parent_id
from issues 
where parent_id is not null
group by parent_id, rgt 
having count(*) > 1

#20 Updated by Bruno Medeiros about 6 years ago

It seems to be fixed after #6555. I couldn't reproduce anymore on demo: http://demo.redmine.org/projects/test6579

Someone could review it and mark as fixed?

#21 Updated by Dani Leni about 6 years ago

"Parent task translation missing: en, activerecord, errors, models, issue, attributes, parent_issue_id, not_a_valid_parent"

I think I still got the issue.
Not sure how its produced.

Happens when trying to update subtask.

Version: Redmine 1.3.2.stable

#22 Updated by Andrey Tatarnikov almost 6 years ago

I think I'm getting same issue with redmine 2.1.2.
I've posted this topic to the board: http://www.redmine.org/boards/2/topics/33878
Sory if I did something wrong.

#23 Updated by Antoine Beaupré over 5 years ago

Note that this also affects projects on Redmine 1.4.

The workaround is to run:

X_DEBIAN_SITEID=koumbit RAILS_ENV=production  ./script/runner 'Project.rebuild!'

(X_DEBIAN_SITEID= is a local hack in the debian package.) This will rebuild the lft and rgt columns that are corrupted. It may mean that the order of projects changes, but the parent/child relationship will stay.

#24 Updated by Robin McKenzie almost 5 years ago

Hi, we're also having this problem. We've tried running Issue.rebuild! but unfortunately it failed with

[FATAL] failed to allocate memory.

This happened with both 1GB and then subsequently 8GB of memory.

We're going to retry with 30GB, but I'm dubious that this will fix it - can anyone offer any advice? (We have just over 100,000 tickets in our Redmine installation.)

Redmine 2.3.0.stable
Ruby 1.9.3
Rails 3.2.13

#25 Updated by Bruno Medeiros almost 5 years ago

How many tickets do you have with problems? If not so much, you can try to edit them and hopefully they will be fixed. Sometimes it worked.

#26 Updated by Toshi MARUYAMA over 4 years ago

  • Related to Defect #6309: missing "not_a_valid_parent" from all the locale files? added

#27 Updated by Toshi MARUYAMA over 4 years ago

  • Related to deleted (Defect #6143: Subtask creation form submission race?)

#28 Updated by Toshi MARUYAMA over 4 years ago

  • Duplicated by Defect #6143: Subtask creation form submission race? added

#29 Updated by Toshi MARUYAMA over 4 years ago

  • Target version changed from Candidate for next minor release to 2.6.0

#30 Updated by Toshi MARUYAMA over 4 years ago

  • % Done changed from 0 to 100

#31 Updated by Toshi MARUYAMA over 4 years ago

awesome_nested_set uses lock since 2.0.2.

https://github.com/collectiveidea/awesome_nested_set/blob/v2.1.6/CHANGELOG#L48

2.0.2
... * Added row locking and fixed some race conditions. [Markus J. Q. Roberts]

I think recent awesome_nested_set (2.1.6) and r12927 fix this issue.

#32 Updated by Bruno Medeiros over 4 years ago

Toshi, let me know when it's available on demo.redmine.org and I can test it to make sure it's ok.

#33 Updated by Toshi MARUYAMA over 4 years ago

Bruno Medeiros wrote:

Toshi, let me know when it's available on demo.redmine.org and I can test it to make sure it's ok.

demo.redmine.org is managed by JPL, not me.
So, I don't know when it is available.

#34 Updated by Toshi MARUYAMA over 4 years ago

  • % Done changed from 100 to 70

#35 Updated by Toshi MARUYAMA over 4 years ago

  • % Done changed from 70 to 100

Committed r13007 to r13011.

#36 Updated by Toshi MARUYAMA almost 4 years ago

#37 Updated by Jean-Philippe Lang almost 4 years ago

  • Subject changed from tree hierachy (lft and rgt) being currupted on (accidentally) multiple submissions of an issue to Tree hierachy being currupted on multiple submissions of an issue
  • Status changed from New to Closed

#38 Updated by Toshi MARUYAMA almost 4 years ago

  • Related to Defect #14335: Issue.rebuild! "lft" and "rgt" are 1 and 2 added

#39 Updated by Toshi MARUYAMA almost 4 years ago

  • Status changed from Closed to New
  • Priority changed from High to Normal
  • Target version deleted (2.6.0)
  • % Done changed from 100 to 80

Sorry, I reopen this issue.
I found some awesome_nested_set bugs.
I will summarise and feed back awesome_nested_set author.

One of bugs is "Issue.rebuild!" set "1" for "lft" and "2" for "rgt" (#14335#note-4).

#40 Updated by Etienne Massip almost 4 years ago

It seems okay to have 1 and 2 if the issue has no hierarchy, what do you expect ?

#41 Updated by Toshi MARUYAMA over 3 years ago

  • Related to Feature #18860: Replace awesome_nested_set gem with a custom implementation of nested sets added

#42 Updated by Toshi MARUYAMA over 3 years ago

  • Related to Defect #15581: Another broken display of subtasks on parent issues added

#43 Updated by Toshi MARUYAMA over 3 years ago

  • Related to Defect #19040: Potential DB deadlocks on concurrent issue creation added

#44 Updated by Toshi MARUYAMA over 3 years ago

  • Related to deleted (Defect #8157: Redmine do not send notification emails if a recipients email address is not valid)

#45 Updated by Toshi MARUYAMA over 3 years ago

  • Target version set to 3.0.0

I think we can close this issue in 3.0.0 because we use explicit lock.

But, I am not sure it is safe we don't use explicit transaction.

#46 Updated by Jean-Philippe Lang over 3 years ago

Toshi MARUYAMA wrote:

But, I am not sure it is safe we don't use explicit transaction.

Toshi, what do you mean? Nested set update is done in AR callbacks that are wrapped in a transaction.

#47 Updated by Toshi MARUYAMA over 3 years ago

OK. I can see "BEGIN" and "COMMIT" on MySQL development.log

$ grep -n -3 -e BEGIN -e COMMIT log/development.log
296-  Issue Load (0.3ms)  SELECT  `issues`.* FROM `issues` WHERE `issues`.`id` = 1 LIMIT 1
297-  CACHE (0.0ms)  SELECT `roles`.* FROM `roles`
298-  CACHE (0.0ms)  SELECT DISTINCT `issue_statuses`.* FROM `issue_statuses` INNER JOIN `workflows` ON `workflows`.`new_status_id` = `issue_statuses`.`id` AND `workflows`.`type` IN ('WorkflowTransition') WHERE `workflows`.`old_status_id` = 1 AND `workflows`.`tracker_id` = 1 AND `workflows`.`role_id` IN (1, 2, 3, 4, 5) AND (author = 1 OR assignee = 0)  [["old_status_id", 1], ["tracker_id", 1]]
299:   (0.1ms)  BEGIN
300-  Issue Load (0.3ms)  SELECT `issues`.* FROM `issues` WHERE `issues`.`parent_id` = 8  ORDER BY `issues`.`lft` ASC
301-  IssueRelation Load (0.1ms)  SELECT `issue_relations`.* FROM `issue_relations` WHERE `issue_relations`.`issue_from_id` = 8
302-  Issue Load (0.2ms)  SELECT `issues`.* FROM `issues` WHERE `issues`.`parent_id` = 1  ORDER BY `issues`.`lft` ASC
--
481-
482-----==_mimepart_54df27595d37a_99d2913ed851659--
483-
484:   (146.6ms)  COMMIT
485-Redirected to http://192.168.11.11:7000/issues/8
486-Completed 302 Found in 832ms (ActiveRecord: 183.8ms)
487-

#48 Updated by Jean-Philippe Lang over 3 years ago

  • Status changed from New to Closed
  • Assignee set to Jean-Philippe Lang
  • Resolution set to Fixed

#49 Updated by Toshi MARUYAMA over 3 years ago

Etienne Massip wrote:

It seems okay to have 1 and 2 if the issue has no hierarchy, what do you expect ?

I have posted #14335#note-10.
As I described on it, Redmine 3.0 "lft" 1 and "rgt" 2 has no problem.

#50 Updated by Toshi MARUYAMA over 3 years ago

  • % Done changed from 80 to 100

#51 Updated by Toshi MARUYAMA about 3 years ago

#52 Updated by Toshi MARUYAMA about 3 years ago

  • Duplicated by Defect #11678: MySQL locks and Redmine collapsing added

#53 Updated by Toshi MARUYAMA over 2 years ago

  • Related to Defect #19344: MySQL 5.6: IssueNestedSetConcurrencyTest#test_concurrency : always fails added

Also available in: Atom PDF