Storm

Merge lp://staging/~therve/storm/flush-order-673666 into lp://staging/storm

  1. flush-order-673666
  2. Merge into trunk
Proposed by Thomas Herve
Status: Needs review
Proposed branch: lp://staging/~therve/storm/flush-order-673666
Merge into: lp://staging/storm
Diff against target: 48 lines (+27/-0)
2 files modified
storm/references.py (+2/-0)
tests/store/base.py (+25/-0)
To merge this branch: bzr merge lp://staging/~therve/storm/flush-order-673666
Reviewer Review Type Date Requested Status
Gustavo Niemeyer Needs Information
Storm Developers Pending
Review via email: mp+40783@code.staging.launchpad.net

Description of the change

The test case was fairly easy to create, and the fix as well. I'm not convinced this is the best fix, but it seems to be reasonable enough.

To post a comment you must log in.
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

[1]

Yeah, this looks quite simple, but it seems to be hiding a bug somewhere else.

The obvious question is why did the pair survive for that long elsewhere, rather than
getting cleaned up when the transaction was flushed?

review: Needs Information
Reply
lp://staging/~therve/storm/flush-order-673666 updated
385. By Thomas Herve

Try another approach

Revision history for this message
Thomas Herve (therve) wrote :

I tried something else: I added an unlink on remove flush. I tried to think hard about the consequences:
 * Can a change go undetected because of that?
 * Can we have a leak because remote_info references local_info?

But I've answered no to both questions, unless I'm missing something.

Reply

Unmerged revisions

385. By Thomas Herve

Try another approach

384. By Thomas Herve

Add a test, and one possible fix.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: