Bug 13800 - Recovery lock bug fixes
Summary: Recovery lock bug fixes
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: CTDB (show other bugs)
Version: 4.9.4
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-21 04:41 UTC by Amitay Isaacs
Modified: 2019-03-28 08:08 UTC (History)
2 users (show)

See Also:


Attachments
Patch for 4.10 (16.86 KB, patch)
2019-03-02 09:43 UTC, Martin Schwenke
amitay: review+
Details
Patch for 4.9 (11.06 KB, patch)
2019-03-02 09:43 UTC, Martin Schwenke
amitay: review+
Details
Additional patch for 4.10 (1.22 KB, application/mbox)
2019-03-19 03:44 UTC, Martin Schwenke
amitay: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Amitay Isaacs 2019-02-21 04:41:32 UTC
There are few issues with recovery lock.

1. Recovery master getting banned and losing election simultaneously will cause double-free.

2. If recovery lock is mis-configured, then the node is stuck trying to take recovery lock.

3. Recovery master keeps waiting indefinitely to take a recovery lock.
Comment 1 Martin Schwenke 2019-03-02 09:43:23 UTC
Created attachment 14893 [details]
Patch for 4.10
Comment 2 Martin Schwenke 2019-03-02 09:43:53 UTC
Created attachment 14894 [details]
Patch for 4.9
Comment 3 Martin Schwenke 2019-03-02 09:47:15 UTC
Patches attached for 4.9 and 4.10.  The included patches applied cleanly from master.

For 4.9 I have dropped the test patches because they didn't apply.  A lot has changed in the test code since then and I don't think the new test is valuable enough to spend time backporting it.  I hand tested a bad recovery lock setting under valgrind and it behaved as expected with all nodes banned.
Comment 4 Amitay Isaacs 2019-03-04 04:18:11 UTC
Hi Karolin,

This is ready for v4-9 and v4-10.
Comment 5 Karolin Seeger 2019-03-04 10:31:29 UTC
(In reply to Amitay Isaacs from comment #4)
Pushed to autobuild-v4-{10,9}-test.
Comment 6 Karolin Seeger 2019-03-06 08:55:34 UTC
(In reply to Karolin Seeger from comment #5)
Pushed to both branches.
Closing out bug report.

Thanks!
Comment 7 Martin Schwenke 2019-03-14 09:32:38 UTC
Reopening this for an extra patch to 4.10.

There is a bug in the test code is only triggered when the test code is installed.  This doesn't happen in autobuild because the tests are run in-tree.

This isn't an urgent fix, so it isn't required for 4.10.0.  However, given that it is only in the test code, it would be safe to sneak it in.  If it passes autobuild then it can do no harm.  ;-)
Comment 8 Martin Schwenke 2019-03-19 03:44:14 UTC
Created attachment 14937 [details]
Additional patch for 4.10
Comment 9 Martin Schwenke 2019-03-19 03:44:49 UTC
Reopened to add an additional patch, details in comment #7.
Comment 10 Amitay Isaacs 2019-03-19 23:17:28 UTC
Hi Karolin,

The additional patch is ready for v4-10.

Thanks.
Comment 11 Karolin Seeger 2019-03-21 12:16:35 UTC
(In reply to Amitay Isaacs from comment #10)
Hi Amitay,

pushed additional patch to autobuild-v4-10-test.

Thanks!
Comment 12 Karolin Seeger 2019-03-28 08:08:06 UTC
(In reply to Karolin Seeger from comment #11)
Pushed additional patch to v4-10-test.
Closing out bug report.

Thanks!