MogileFS 2.61 and beyond

133 views
Skip to first unread message

dormando

unread,
Mar 30, 2012, 6:51:42 PM3/30/12
to mog...@googlegroups.com
Yo,

I'm going to reorganize my roadmap a bit and post it, however... In the
immediate feature, I want to add no new features until we can get past a
testing issue.

That is, that the testing sucks. MogileFS is big and complex and the test
coverage is missing a lot. I've also had a bad habit of fixing bugs but
not adding tests for them.

FSCK still has no automated tests. Many of the issues I ran into with
stabilizing the core rewrite in 2.50 stemmed from lacking tests in corner
cases.

So before we move on and add new features, I want to do a lot of work on
the test suite. I invite any and all of you to help out. We need to go
back through all of the crap I fixed in 2.50, as well as visually
reviewing the code or looking at a Coverage graph from a test run, and
fill in a bunch.

I will accept bugfixes and changes on the Checksums work in the meantime.
Ideally bugfixes come with decent tests as well.

After I feel a lot more confident with the test suite, we can accelerate
development again. It's been almost a year since the 2.50 core rewrite,
and 2012 is going to be a busy one for MFS!

thanks!
-Dormando

扶凯

unread,
Mar 31, 2012, 6:29:24 AM3/31/12
to mog...@googlegroups.com
2.50 core rewrite...
Great. Can not use anyevent to handle the event, and then use the message queue to isolate the Worker, multi-network can also be remote calls, it is convenient and more easily extended.

在 2012年3月31日星期六UTC+8上午6时51分42秒,Dormando写道:
在 2012年3月31日星期六UTC+8上午6时51分42秒,Dormando写道:

Eric Wong

unread,
Apr 18, 2012, 3:35:40 PM4/18/12
to mog...@googlegroups.com
dormando <dorm...@rydia.net> wrote:
> FSCK still has no automated tests.

I'm working on this (first without changing existing code), should
hopefully be progress soon. Should make it easier to make
performance and locking changes to fsck/delete/replicate and avoid
races in there.

dormando

unread,
Apr 18, 2012, 3:39:14 PM4/18/12
to mog...@googlegroups.com

Huzzah!

Feel free to ask me stuff about Fsck. I've spent a lot of time in that
code and deleted half of it already, so there's probably a lot of code
that doesn't seem to make sense.

Eric Wong

unread,
Apr 18, 2012, 5:00:25 PM4/18/12
to mog...@googlegroups.com
dormando <dorm...@rydia.net> wrote:
> Huzzah!
>
> Feel free to ask me stuff about Fsck. I've spent a lot of time in that
> code and deleted half of it already, so there's probably a lot of code
> that doesn't seem to make sense.

OK. Writing tests helped me make sense of most of it :)

The behavior I'm not sure about are annotated with XXX below:

From b29119f0dd32f1a9e158646a7fd2d4aabe2c5f1a Mon Sep 17 00:00:00 2001
From: Eric Wong <normal...@yhbt.net>
Date: Wed, 18 Apr 2012 13:54:00 -0700
Subject: [PATCH] tests: add test for fsck functionality

Before we make changes to the fsck code, we should ensure
we don't break existing use cases.

Behavior I'm uncertain about is documented with "XXX".
---
Pushed to: git://bogomips.org/MogileFS-Server.git fsck-tests

t/60-fsck.t | 292 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 292 insertions(+)
create mode 100644 t/60-fsck.t

diff --git a/t/60-fsck.t b/t/60-fsck.t
new file mode 100644
index 0000000..b03e085
--- /dev/null
+++ b/t/60-fsck.t
@@ -0,0 +1,292 @@
+# -*-perl-*-
+# some of the comments match the comments in MogileFS/Worker/Fsck.pm
+# _exactly_ for reference purposes
+use strict;
+use warnings;
+use Test::More;
+use FindBin qw($Bin);
+use Time::HiRes qw(sleep);
+use MogileFS::Server;
+use MogileFS::Test;
+use HTTP::Request;
+find_mogclient_or_skip();
+use MogileFS::Admin;
+
+my $sto = eval { temp_store(); };
+if (!$sto) {
+ plan skip_all => "Can't create temporary test database: $@";
+ exit 0;
+}
+
+use File::Temp;
+my %mogroot;
+$mogroot{1} = File::Temp::tempdir( CLEANUP => 1 );
+$mogroot{2} = File::Temp::tempdir( CLEANUP => 1 );
+my $dev2host = { 1 => 1, 2 => 2, };
+foreach (sort { $a <=> $b } keys %$dev2host) {
+ my $root = $mogroot{$dev2host->{$_}};
+ mkdir("$root/dev$_") or die "Failed to create dev$_ dir: $!";
+}
+
+my $ms1 = create_mogstored("127.0.1.1", $mogroot{1});
+ok($ms1, "got mogstored1");
+my $ms2 = create_mogstored("127.0.1.2", $mogroot{2});
+ok($ms1, "got mogstored2");
+
+while (! -e "$mogroot{1}/dev1/usage" &&
+ ! -e "$mogroot{2}/dev2/usage") {
+ print "Waiting on usage...\n";
+ sleep(.25);
+}
+
+my $tmptrack = create_temp_tracker($sto);
+ok($tmptrack);
+
+my $admin = IO::Socket::INET->new(PeerAddr => '127.0.0.1:7001');
+$admin or die "failed to create admin socket: $!";
+my $moga = MogileFS::Admin->new(hosts => [ "127.0.0.1:7001" ]);
+my $mogc = MogileFS::Client->new(
+ domain => "testdom",
+ hosts => [ "127.0.0.1:7001" ],
+ );
+my $be = $mogc->{backend}; # gross, reaching inside of MogileFS::Client
+
+# test some basic commands to backend
+ok($tmptrack->mogadm("domain", "add", "testdom"), "created test domain");
+ok($tmptrack->mogadm("class", "add", "testdom", "2copies", "--mindevcount=2"), "created 2copies class in testdom");
+ok($tmptrack->mogadm("class", "add", "testdom", "1copy", "--mindevcount=1"), "created 1copy class in testdom");
+
+ok($tmptrack->mogadm("host", "add", "hostA", "--ip=127.0.1.1", "--status=alive"), "created hostA");
+ok($tmptrack->mogadm("host", "add", "hostB", "--ip=127.0.1.2", "--status=alive"), "created hostB");
+
+ok($tmptrack->mogadm("device", "add", "hostA", 1), "created dev1 on hostA");
+ok($tmptrack->mogadm("device", "add", "hostB", 2), "created dev2 on hostB");
+
+sub wait_for_monitor {
+ my $be = shift;
+ my $was = $be->{timeout}; # can't use local on phash :(
+ $be->{timeout} = 10;
+ ok($be->do_request("clear_cache", {}), "waited for monitor")
+ or die "Failed to wait for monitor";
+ ok($be->do_request("clear_cache", {}), "waited for monitor")
+ or die "Failed to wait for monitor";
+ $be->{timeout} = $was;
+}
+
+sub wait_for_empty_queue {
+ my ($table, $dbh) = @_;
+ my $limit = 600;
+ my $delay = 0.1;
+ my $i = $limit;
+ my $count;
+ while ($i > 0) {
+ $count = $dbh->selectrow_array("SELECT COUNT(*) from $table");
+ return if ($count == 0);
+ sleep $delay;
+ }
+ my $time = $delay * $limit;
+ die "$table is not empty after ${time}s!";
+}
+
+sub full_fsck {
+ my ($tmptrack, $dbh) = @_;
+
+ # this should help prevent race conditions:
+ wait_for_empty_queue("file_to_queue", $dbh);
+
+ ok($tmptrack->mogadm("fsck", "stop"), "stop fsck");
+ ok($tmptrack->mogadm("fsck", "clearlog"), "clear fsck log");
+ ok($tmptrack->mogadm("fsck", "reset"), "reset fsck");
+ ok($tmptrack->mogadm("fsck", "start"), "started fsck");
+}
+
+wait_for_monitor($be);
+
+my ($req, $rv, %opts, @paths, @fsck_log, $info);
+my $ua = LWP::UserAgent->new;
+my $key = "testkey";
+my $dbh = $sto->dbh;
+
+use Data::Dumper;
+
+# upload a file and wait for replica to appear
+{
+ my $fh = $mogc->new_file($key, "1copy");
+ print $fh "hello\n";
+ ok(close($fh), "closed file");
+}
+
+# first obvious fucked-up case: no devids even presumed to exist.
+{
+ $info = $mogc->file_info($key);
+ is($info->{devcount}, 1, "ensure devcount is correct at start");
+
+ # ensure repl queue is empty before destroying file_on
+ wait_for_empty_queue("file_to_replicate", $dbh);
+
+ is($dbh->do("DELETE FROM file_on"), 1, "delete $key from file_on table");
+ full_fsck($tmptrack, $dbh);
+ do {
+ @fsck_log = $sto->fsck_log_rows;
+ } while (scalar(@fsck_log) < 3 && sleep(0.1));
+
+ wait_for_empty_queue("file_to_queue", $dbh);
+ @fsck_log = $sto->fsck_log_rows;
+
+ my $nopa = $fsck_log[0];
+ is($nopa->{evcode}, "NOPA", "evcode for no paths logged");
+
+ # entering "desperate" mode
+ my $srch = $fsck_log[1];
+ is($srch->{evcode}, "SRCH", "evcode for start search logged");
+
+ # wow, we actually found it!
+ my $fond = $fsck_log[2];
+ is($fond->{evcode}, "FOND", "evcode for start search logged");
+
+ $info = $mogc->file_info($key);
+ is($info->{devcount}, 1, "ensure devcount is correct at fsck end");
+ @paths = $mogc->get_paths($key);
+ is(scalar(@paths), 1, "get_paths returns correctly at fsck end");
+}
+
+# update class to require 2copies and have fsck fix it
+{
+ @paths = $mogc->get_paths($key);
+ is(scalar(@paths), 1, "only one path exists before fsck");
+
+ # _NOT_ using "updateclass" command since that enqueues for replication
+ my $fid = MogileFS::FID->new($info->{fid});
+ my $classid_2copies = $dbh->selectrow_array("SELECT classid FROM class WHERE dmid = ? AND classname = ?", undef, $fid->dmid, "2copies");
+ is($fid->update_class(classid => $classid_2copies), 1, "classid updated");
+
+ full_fsck($tmptrack, $dbh);
+
+ do {
+ @paths = $mogc->get_paths($key);
+ } while (scalar(@paths) == 1 and sleep(0.1));
+ is(scalar(@paths), 2, "replicated from fsck");
+
+ $info = $mogc->file_info($key);
+ is($info->{devcount}, 2, "ensure devcount is updated by replicate");
+
+ do {
+ @fsck_log = $sto->fsck_log_rows;
+ } while (scalar(@fsck_log) == 0 and sleep(10));
+
+ my $povi = $fsck_log[0];
+ is($povi->{evcode}, "POVI", "policy violation logged by fsck");
+
+ my $repl = $fsck_log[1];
+ is($repl->{evcode}, "REPL", "replication request logged by fsck");
+}
+
+# wrong devcount in file column, but otherwise everything is OK
+{
+ foreach my $wrong_devcount (13, 0, 1) {
+ is($dbh->do("UPDATE file SET devcount = ? WHERE fid = ?", undef, $wrong_devcount, $info->{fid}), 1, "set improper devcount");
+
+ $info = $mogc->file_info($key);
+ is($info->{devcount}, $wrong_devcount, "devcount is set to $wrong_devcount");
+
+ full_fsck($tmptrack, $dbh);
+
+ do {
+ $info = $mogc->file_info($key);
+ } while ($info->{devcount} == $wrong_devcount && sleep(0.1));
+ is($info->{devcount}, 2, "devcount is corrected by fsck");
+
+ # XXX POVI gets logged here, but BCNT might be more correct...
+ wait_for_empty_queue("file_to_queue", $dbh);
+ @fsck_log = $sto->fsck_log_rows;
+ is($fsck_log[0]->{evcode}, "POVI", "policy violation logged");
+ }
+}
+
+# nuke a file from disk but keep the file_on row
+{
+ @paths = $mogc->get_paths($key);
+ is(scalar(@paths), 2, "two paths returned from get_paths");
+ $rv = $ua->delete($paths[0]);
+ ok($rv->is_success, "DELETE successful");
+
+ full_fsck($tmptrack, $dbh);
+ do {
+ @fsck_log = $sto->fsck_log_rows;
+ } while (scalar(@fsck_log) < 2 && sleep(0.1));
+
+ my $miss = $fsck_log[0];
+ is($miss->{evcode}, "MISS", "missing file logged by fsck");
+
+ my $repl = $fsck_log[1];
+ is($repl->{evcode}, "REPL", "replication request logged by fsck");
+
+ wait_for_empty_queue("file_to_replicate", $dbh);
+
+ @paths = $mogc->get_paths($key);
+ is(scalar(@paths), 2, "two paths returned from get_paths");
+ foreach my $path (@paths) {
+ $rv = $ua->get($path);
+ is($rv->content, "hello\n", "GET successful on restored path");
+ }
+}
+
+# change the length of a file from disk and have fsck correct it
+{
+ @paths = $mogc->get_paths($key);
+ is(scalar(@paths), 2, "two paths returned from get_paths");
+ $req = HTTP::Request->new(PUT => $paths[0]);
+ $req->content("hello\r\n");
+ $rv = $ua->request($req);
+ ok($rv->is_success, "PUT successful");
+
+ full_fsck($tmptrack, $dbh);
+ do {
+ @fsck_log = $sto->fsck_log_rows;
+ } while (scalar(@fsck_log) < 2 && sleep(0.1));
+
+ my $blen = $fsck_log[0];
+ is($blen->{evcode}, "BLEN", "missing file logged by fsck");
+
+ my $repl = $fsck_log[1];
+ is($repl->{evcode}, "REPL", "replication request logged by fsck");
+
+ wait_for_empty_queue("file_to_replicate", $dbh);
+
+ @paths = $mogc->get_paths($key);
+ is(scalar(@paths), 2, "two paths returned from get_paths");
+ foreach my $path (@paths) {
+ $rv = $ua->get($path);
+ is($rv->content, "hello\n", "GET successful on restored path");
+ }
+}
+
+# nuke a file completely and irreparably
+{
+ @paths = $mogc->get_paths($key);
+ is(scalar(@paths), 2, "two paths returned from get_paths");
+ foreach my $path (@paths) {
+ $rv = $ua->delete($path);
+ ok($rv->is_success, "DELETE successful");
+ }
+
+ full_fsck($tmptrack, $dbh);
+ do {
+ @fsck_log = $sto->fsck_log_rows;
+ } while (scalar(@fsck_log) < 4 && sleep(0.1));
+
+ is($fsck_log[0]->{evcode}, "MISS", "missing file logged for first path");
+ is($fsck_log[1]->{evcode}, "MISS", "missing file logged for second path");
+ is($fsck_log[2]->{evcode}, "SRCH", "desperate search attempt logged");
+ is($fsck_log[3]->{evcode}, "GONE", "inability to fix FID logged");
+
+ wait_for_empty_queue("file_to_queue", $dbh);
+ $info = $mogc->file_info($key);
+
+ # XXX devcount probably needs to be updated on GONE
+ # is($info->{devcount}, 2, "devcount updated to zero");
+ @paths = $mogc->get_paths($key);
+ is(scalar(@paths), 0, "get_paths returns nothing");
+}
+
+done_testing();
--
Eric Wong

Eric Wong

unread,
Apr 18, 2012, 5:16:02 PM4/18/12
to mog...@googlegroups.com
The LWP::UserAgent module found in my Debian 6.0 installation
does not have a "delete" convenience wrapper.
---
t/60-fsck.t | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

Also pushed to the "fsck-tests" branch on
git://bogomips.org/MogileFS-Server

diff --git a/t/60-fsck.t b/t/60-fsck.t
index b03e085..8b5c5f4 100644
--- a/t/60-fsck.t
+++ b/t/60-fsck.t
@@ -207,7 +207,8 @@ use Data::Dumper;
{
@paths = $mogc->get_paths($key);


is(scalar(@paths), 2, "two paths returned from get_paths");

- $rv = $ua->delete($paths[0]);
+ $req = HTTP::Request->new(DELETE => $paths[0]);
+ $rv = $ua->request($req);


ok($rv->is_success, "DELETE successful");

full_fsck($tmptrack, $dbh);
@@ -266,7 +267,8 @@ use Data::Dumper;
@paths = $mogc->get_paths($key);


is(scalar(@paths), 2, "two paths returned from get_paths");

foreach my $path (@paths) {
- $rv = $ua->delete($path);
+ $req = HTTP::Request->new(DELETE => $path);


+ $rv = $ua->request($req);

ok($rv->is_success, "DELETE successful");
}

--

Daniel Frett

unread,
Apr 19, 2012, 1:42:28 AM4/19/12
to mog...@googlegroups.com
delete and put convenience wrappers were only added with LWP::UserAgent 6.04 released in February, I ran into the same issue with a current project at work.

-Daniel

tariq wali

unread,
Apr 20, 2012, 7:02:47 AM4/20/12
to mog...@googlegroups.com
how safe is to upgrade from  2.57 to 2.61 ? Any important measures that need to be considered before the upgrade ?
--
Tariq Wali.

dormando

unread,
Apr 20, 2012, 1:46:20 PM4/20/12
to mog...@googlegroups.com
2.60 is the latest version.

As always, there's the upgrading guide:
http://code.google.com/p/mogilefs/wiki/Upgrading#2.60

dormando

unread,
Apr 21, 2012, 4:25:31 AM4/21/12
to mog...@googlegroups.com
> OK. Writing tests helped me make sense of most of it :)

Yay! :)

I've pushed this into the "next" tree, so it's tentative while we work on
it. I have some comments I'll throw inline below..

> The behavior I'm not sure about are annotated with XXX below:
>
> From b29119f0dd32f1a9e158646a7fd2d4aabe2c5f1a Mon Sep 17 00:00:00 2001
> From: Eric Wong <normal...@yhbt.net>
> Date: Wed, 18 Apr 2012 13:54:00 -0700
> Subject: [PATCH] tests: add test for fsck functionality
>
> Before we make changes to the fsck code, we should ensure
> we don't break existing use cases.
>
> Behavior I'm uncertain about is documented with "XXX".
> ---
> Pushed to: git://bogomips.org/MogileFS-Server.git fsck-tests
>
> t/60-fsck.t | 292 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 292 insertions(+)
> create mode 100644 t/60-fsck.t

Don't forget to add t/60-fsck.t to the MANIFEST and such.

> +sub full_fsck {
> + my ($tmptrack, $dbh) = @_;
> +
> + # this should help prevent race conditions:
> + wait_for_empty_queue("file_to_queue", $dbh);
> +
> + ok($tmptrack->mogadm("fsck", "stop"), "stop fsck");
> + ok($tmptrack->mogadm("fsck", "clearlog"), "clear fsck log");
> + ok($tmptrack->mogadm("fsck", "reset"), "reset fsck");
> + ok($tmptrack->mogadm("fsck", "start"), "started fsck");
> +}

Also need to start start/stop/resume work, since I've broken those several
times. Use server settings to crank the speed down to 1 per second and
stop/etc?

Might also be nice to verify the stats output would be correct by
examining the server setting variables. I'd just fixed a bug where it
wasn't finding the max rows until you'd run the fsck through once. So it'd
be on file "500 out of 0"

> + is($dbh->do("DELETE FROM file_on"), 1, "delete $key from file_on table");

maybe use $sto for these things?

> + # _NOT_ using "updateclass" command since that enqueues for replication
> + my $fid = MogileFS::FID->new($info->{fid});
> + my $classid_2copies = $dbh->selectrow_array("SELECT classid FROM class WHERE dmid = ? AND classname = ?", undef, $fid->dmid, "2copies");
> + is($fid->update_class(classid => $classid_2copies), 1, "classid updated");

same?

I had some other comments but I talked myself out of them.

Other ideas to test:
- fsck fids that exist on a dead device (someone marks dev dead but
starts a fsck before it's drained). This used to not work and was fixed at
some point.
- fsck avoids down hosts/devices properly
- too braindead to think of others right now.

Thanks again for your awesome work!
-Dormando

dormando

unread,
Apr 21, 2012, 6:33:22 PM4/21/12
to mog...@googlegroups.com
I'm sorry, I don't understand what you are saying here :)

Eric Wong

unread,
Apr 21, 2012, 11:17:45 PM4/21/12
to mog...@googlegroups.com
Updates to the "fsck-tests" branch pushed, I've rebased it
against "next" right now.

(git://bogomips.org/MogileFS-Server)

dormando <dorm...@rydia.net> wrote:
> Don't forget to add t/60-fsck.t to the MANIFEST and such.

Trivial, pushed as commit 644be5043b3143a9f46506b48f3a7ec47c2f55d9

> > + is($dbh->do("DELETE FROM file_on"), 1, "delete $key from file_on table");
>
> maybe use $sto for these things?
>
> > + # _NOT_ using "updateclass" command since that enqueues for replication
> > + my $fid = MogileFS::FID->new($info->{fid});
> > + my $classid_2copies = $dbh->selectrow_array("SELECT classid FROM class WHERE dmid = ? AND classname = ?", undef, $fid->dmid, "2copies");
> > + is($fid->update_class(classid => $classid_2copies), 1, "classid updated");
>
> same?

OK, was a bit hasty with the original and didn't find API methods :x
Will work on the rest of the stuff later.

From 8789f2f89a7192355a33684d425a2af11e16f376 Mon Sep 17 00:00:00 2001
From: Eric Wong <normal...@yhbt.net>
Date: Sun, 22 Apr 2012 03:08:51 +0000
Subject: [PATCH] tests: fsck test use MogileFS::Store API when possible

No point in using DBI directly if a task can be done directly
via the MogileFS::Store API.

Noticed-by: dormando
---
t/60-fsck.t | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/t/60-fsck.t b/t/60-fsck.t
index 8b5c5f4..0d9bc2b 100644
--- a/t/60-fsck.t
+++ b/t/60-fsck.t
@@ -124,7 +124,11 @@ use Data::Dumper;
# ensure repl queue is empty before destroying file_on
wait_for_empty_queue("file_to_replicate", $dbh);

- is($dbh->do("DELETE FROM file_on"), 1, "delete $key from file_on table");
+ my @devids = $sto->fid_devids($info->{fid});
+ is(scalar(@devids), 1, "devids retrieved");
+ is($sto->remove_fidid_from_devid($info->{fid}, $devids[0]), 1,
+ "delete $key from file_on table");
+
full_fsck($tmptrack, $dbh);
do {
@fsck_log = $sto->fsck_log_rows;
@@ -157,7 +161,7 @@ use Data::Dumper;

# _NOT_ using "updateclass" command since that enqueues for replication
my $fid = MogileFS::FID->new($info->{fid});
- my $classid_2copies = $dbh->selectrow_array("SELECT classid FROM class WHERE dmid = ? AND classname = ?", undef, $fid->dmid, "2copies");
+ my $classid_2copies = $sto->get_classid_by_name($fid->dmid, "2copies");
is($fid->update_class(classid => $classid_2copies), 1, "classid updated");

full_fsck($tmptrack, $dbh);
--
Eric Wong

Eric Wong

unread,
Apr 29, 2012, 5:15:26 AM4/29/12
to mog...@googlegroups.com
dormando <dorm...@rydia.net> wrote:
> Other ideas to test:
> - fsck fids that exist on a dead device (someone marks dev dead but
> starts a fsck before it's drained). This used to not work and was fixed at
> some point.
> - fsck avoids down hosts/devices properly

I've pushed this out to the "fsck-tests" branch on
git://bogomips.org/MogileFS-Server.git

From ccab7b41874e85ad5a450517e7ba8b6f095fe9c3 Mon Sep 17 00:00:00 2001
From: Eric Wong <normal...@yhbt.net>
Date: Fri, 27 Apr 2012 03:01:11 +0000
Subject: [PATCH] t/60-fsck: additional test cases

* ensure fsck can handle a stall from an unresponsive mogstored

* ensure over-replicated files are cleaned up

* ensure fsck can work correctly with dead devices if it beats
reaper to an FID
---
t/60-fsck.t | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 161 insertions(+), 3 deletions(-)

diff --git a/t/60-fsck.t b/t/60-fsck.t
index 31bdb64..eef4192 100644
--- a/t/60-fsck.t
+++ b/t/60-fsck.t
@@ -22,7 +22,8 @@ use File::Temp;
my %mogroot;
$mogroot{1} = File::Temp::tempdir( CLEANUP => 1 );
$mogroot{2} = File::Temp::tempdir( CLEANUP => 1 );
-my $dev2host = { 1 => 1, 2 => 2, };
+$mogroot{3} = File::Temp::tempdir( CLEANUP => 1 );
+my $dev2host = { 1 => 1, 2 => 2, 3 => 2 };
foreach (sort { $a <=> $b } keys %$dev2host) {
my $root = $mogroot{$dev2host->{$_}};
mkdir("$root/dev$_") or die "Failed to create dev$_ dir: $!";
@@ -33,8 +34,9 @@ ok($ms1, "got mogstored1");
my $ms2 = create_mogstored("127.0.1.2", $mogroot{2});
ok($ms1, "got mogstored2");

-while (! -e "$mogroot{1}/dev1/usage" &&
- ! -e "$mogroot{2}/dev2/usage") {
+while (! -e "$mogroot{1}/dev1/usage" ||
+ ! -e "$mogroot{2}/dev2/usage" ||
+ ! -e "$mogroot{2}/dev3/usage") {
print "Waiting on usage...\n";
sleep(.25);
}
@@ -295,4 +297,160 @@ use Data::Dumper;
is(scalar(@paths), 0, "get_paths returns nothing");
}

+# upload a file and wait for replica to appear
+{
+ my $fh = $mogc->new_file($key, "2copies");
+ print $fh "hello\n";
+ ok(close($fh), "closed file");
+
+ do {
+ $info = $mogc->file_info($key);
+ } while ($info->{devcount} < 2);
+ is($info->{devcount}, 2, "ensure devcount is correct at start");
+}
+
+# stall fsck with a non-responsive host
+{
+ is(kill("STOP", $ms1->pid), 1, "send SIGSTOP to mogstored1");
+ wait_for_monitor($be);
+
+ my $delay = 10;
+ ok($tmptrack->mogadm("fsck", "start"), "started fsck, sleeping ${delay}s");
+ sleep $delay;
+ is($sto->file_queue_length(MogileFS::Config::FSCK_QUEUE), 1, "fsck queue still blocked");
+}
+
+# resume fsck when host is responsive again
+{
+ is(kill("CONT", $ms1->pid), 1, "send SIGCONT to mogstored1");
+ wait_for_monitor($be);
+
+ # force fsck to wakeup and do work again
+ my $now = $sto->unix_timestamp;
+ is($sto->dbh->do("UPDATE file_to_queue SET nexttry = $now"), 1, "unblocked fsck queue");
+ ok($admin->syswrite("!to fsck :wake_up\n"), "force fsck to wake up");
+ ok($admin->getline, "got wakeup response 1");
+ ok($admin->getline, "got wakeup response 2");
+
+ foreach my $i (1..30) {
+ last if $sto->file_queue_length(MogileFS::Config::FSCK_QUEUE) == 0;
+
+ sleep 1;
+ }
+
+ is($sto->file_queue_length(MogileFS::Config::FSCK_QUEUE), 0, "fsck queue emptied");
+}
+
+# upload a file and wait for replica to appear
+{
+ my $fh = $mogc->new_file($key, "2copies");
+ print $fh "hello\n";
+ ok(close($fh), "closed file");
+
+ do {
+ $info = $mogc->file_info($key);
+ } while ($info->{devcount} < 2);
+ is($info->{devcount}, 2, "ensure devcount is correct at start");
+}
+
+# stall fsck with a non-responsive host
+# resume fsck when host is responsive again
+{
+ is(kill("STOP", $ms1->pid), 1, "send SIGSTOP to mogstored1 to stall");
+ wait_for_monitor($be);
+
+ my $delay = 10;
+ ok($tmptrack->mogadm("fsck", "start"), "started fsck, sleeping ${delay}s");
+ sleep $delay;
+ is($sto->file_queue_length(MogileFS::Config::FSCK_QUEUE), 1, "fsck queue still blocked");
+
+ is(kill("CONT", $ms1->pid), 1, "send SIGCONT to mogstored1 to resume");
+ wait_for_monitor($be);
+
+ # force fsck to wakeup and do work again
+ my $now = $sto->unix_timestamp;
+ is($sto->dbh->do("UPDATE file_to_queue SET nexttry = $now"), 1, "unblocked fsck queue");
+ ok($admin->syswrite("!to fsck :wake_up\n"), "force fsck to wake up");
+ ok($admin->getline, "got wakeup response 1");
+ ok($admin->getline, "got wakeup response 2");
+
+ foreach my $i (1..30) {
+ last if $sto->file_queue_length(MogileFS::Config::FSCK_QUEUE) == 0;
+
+ sleep 1;
+ }
+
+ is($sto->file_queue_length(MogileFS::Config::FSCK_QUEUE), 0, "fsck queue emptied");
+}
+
+# cleanup over-replicated file
+{
+ $info = $mogc->file_info($key);
+ is($info->{devcount}, 2, "ensure devcount is correct at start");
+
+ # _NOT_ using "updateclass" command since that enqueues for replication
+ my $fid = MogileFS::FID->new($info->{fid});
+ my $classid_1copy = $sto->get_classid_by_name($fid->dmid, "1copy");
+ is($fid->update_class(classid => $classid_1copy), 1, "classid updated");
+
+ full_fsck($tmptrack, $dbh);
+
+ sleep(1) while $mogc->file_info($key)->{devcount} == 2;
+ is($mogc->file_info($key)->{devcount}, 1, "too-happy file cleaned up");
+
+ @fsck_log = $sto->fsck_log_rows;
+ is($fsck_log[0]->{evcode}, "POVI", "policy violation logged");
+
+ # replicator is requested to delete too-happy file
+ is($fsck_log[1]->{evcode}, "REPL", "replication request logged");
+}
+
+# kill a device and replace it with another, without help from reaper
+# this test may become impossible if monitor + reaper are merged...
+{
+ ok($mogc->update_class($key, "2copies"), "request 2 replicas again");
+ do {
+ $info = $mogc->file_info($key);
+ } while ($info->{devcount} < 2);
+ is($info->{devcount}, 2, "ensure devcount is correct at start");
+ wait_for_empty_queue("file_to_replicate", $dbh);
+
+ $admin->syswrite("!jobs\n");
+ my $reaper_pid;
+
+ while (my $line = $admin->getline) {
+ last if $line =~ /^\.\r?\n/;
+
+ if ($line =~ /^reaper pids (\d+)\r?\n/) {
+ $reaper_pid = $1;
+ }
+ }
+ ok($reaper_pid, "got pid of reaper");
+
+ # reaper is watchdog is 240s, and it wakes up in 5s intervals right now
+ # so we should be safe from watchdog for now...
+ ok(kill("STOP", $reaper_pid), "stopped reaper from running");
+
+ ok($tmptrack->mogadm("device", "mark", "hostB", 2, "dead"), "mark dev2 as dead");
+ ok($tmptrack->mogadm("device", "add", "hostB", 3), "created dev3 on hostB");
+ wait_for_monitor($be);
+
+ full_fsck($tmptrack, $dbh);
+
+ sleep 1 while MogileFS::Config->server_setting("fsck_host");
+
+ foreach my $i (1..30) {
+ last if $sto->file_queue_length(MogileFS::Config::FSCK_QUEUE) == 0;
+ sleep 1;
+ }
+ is($sto->file_queue_length(MogileFS::Config::FSCK_QUEUE), 0, "fsck queue empty");
+
+ # fsck should've corrected what reaper missed
+ @fsck_log = $sto->fsck_log_rows;
+ is(scalar(@fsck_log), 1, "fsck log populated");
+ is($fsck_log[0]->{evcode}, "REPL", "replication enqueued");
+
+ ok(kill("CONT", $reaper_pid), "resumed reaper");
+}
+
done_testing();
--

Eric Wong

unread,
May 2, 2012, 5:15:05 AM5/2/12
to mog...@googlegroups.com
We need to ensure fsck can resume and returns sane
stats output.
---
Also pushed to the fsck-tests branch of
git://bogomips.org/MogileFS-Server.git

Under Postgres, I've gotten some sporadic failures that I can't
reproduce consistently.

not ok 168 - moved to next FID
# Failed test 'moved to next FID'
# at t/60-fsck.t line 489.
# got: '13'
# expected: anything else
not ok 174 - fsck continued to higher FID
# Failed test 'fsck continued to higher FID'
# at t/60-fsck.t line 507.
# '13'
# >
# '13'

No issues with SQLite and MySQL.

dormando <dorm...@rydia.net> wrote:
> Also need to start start/stop/resume work, since I've broken those several
> times. Use server settings to crank the speed down to 1 per second and
> stop/etc?
>
> Might also be nice to verify the stats output would be correct by
> examining the server setting variables. I'd just fixed a bug where it
> wasn't finding the max rows until you'd run the fsck through once. So it'd
> be on file "500 out of 0"

t/60-fsck.t | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)

diff --git a/t/60-fsck.t b/t/60-fsck.t
index 8b18d5c..3a2bb00 100644
--- a/t/60-fsck.t
+++ b/t/60-fsck.t
@@ -452,4 +452,59 @@ use Data::Dumper;
ok(kill("CONT", $reaper_pid), "resumed reaper");
}

+{
+ foreach my $i (1..10) {
+ my $fh = $mogc->new_file("k$i", "1copy");
+ print $fh "$i\n";
+ ok(close($fh), "closed file ($i)");
+ }
+ $info = $mogc->file_info("k10");
+
+ ok($tmptrack->mogadm("settings", "set", "queue_rate_for_fsck", 1), "set queue_rate_for_fsck to 1");
+ ok($tmptrack->mogadm("settings", "set", "queue_size_for_fsck", 1), "set queue_size_for_fsck to 1");
+ wait_for_monitor($be);
+
+ ok($tmptrack->mogadm("fsck", "start"), "start fsck with slow queue rate");
+
+ ok(MogileFS::Config->server_setting("fsck_host"), "fsck_host set");
+ is(MogileFS::Config->server_setting("fsck_fid_at_end"), $info->{fid}, "fsck_fid_at_end matches");
+
+ my $highest = undef;
+ foreach my $i (1..100) {
+ $highest = MogileFS::Config->server_setting("fsck_highest_fid_checked");
+ last if defined $highest;
+ sleep 0.1;
+ }
+ ok($highest, "fsck_highest_fid_checked is set");
+ like($highest, qr/\A\d+\z/, "fsck_highest_fid_checked is a digit");
+
+ # wait for something to get fscked
+ foreach my $i (1..100) {
+ last if MogileFS::Config->server_setting("fsck_highest_fid_checked") != $highest;
+ sleep 0.1;
+ }
+
+ my $old_highest = $highest;
+ $highest = MogileFS::Config->server_setting("fsck_highest_fid_checked");
+ isnt($highest, $old_highest, "moved to next FID");
+
+ ok($tmptrack->mogadm("fsck", "stop"), "stop fsck");
+ ok(! MogileFS::Config->server_setting("fsck_host"), "fsck_host unset");
+ is(MogileFS::Config->server_setting("fsck_fid_at_end"), $info->{fid}, "fsck_fid_at_end matches");
+
+ # resume paused fsck
+ ok($tmptrack->mogadm("fsck", "start"), "restart fsck");
+ $highest = MogileFS::Config->server_setting("fsck_highest_fid_checked");
+ cmp_ok($highest, '>=', $old_highest, "fsck resumed without resetting fsck_highest_fid_checked");
+
+ # wait for something to get fscked
+ foreach my $i (1..200) {
+ last if MogileFS::Config->server_setting("fsck_highest_fid_checked") != $highest;
+ sleep 0.1;
+ }
+
+ $highest = MogileFS::Config->server_setting("fsck_highest_fid_checked");
+ cmp_ok($highest, '>', $old_highest, "fsck continued to higher FID");
+}
+
done_testing();
--
Eric Wong

Eric Wong

unread,
May 9, 2012, 8:32:32 PM5/9/12
to mog...@googlegroups.com
Eric Wong <normal...@yhbt.net> wrote:
> Also pushed to the fsck-tests branch of
> git://bogomips.org/MogileFS-Server.git
>
> Under Postgres, I've gotten some sporadic failures that I can't
> reproduce consistently.

Should be fixed with my latest change. I've looped these on
SQLite, MySQL, and Postgres on a box for ~48 hours now.

Eric Wong (2):
t/60-fsck: fix typo resulting in useless check
t/60-fsck: fix potential race conditions

Eric Wong

unread,
May 9, 2012, 8:33:13 PM5/9/12
to mog...@googlegroups.com
We checked the incorrect return value, so the second
mogstored failing would've gone unnoticed.
---
t/60-fsck.t | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/60-fsck.t b/t/60-fsck.t
index e33b8da..98a7bad 100644
--- a/t/60-fsck.t
+++ b/t/60-fsck.t
@@ -32,7 +32,7 @@ foreach (sort { $a <=> $b } keys %$dev2host) {
my $ms1 = create_mogstored("127.0.1.1", $mogroot{1});
ok($ms1, "got mogstored1");
my $ms2 = create_mogstored("127.0.1.2", $mogroot{2});
-ok($ms1, "got mogstored2");
+ok($ms2, "got mogstored2");

while (! -e "$mogroot{1}/dev1/usage" ||
! -e "$mogroot{2}/dev2/usage" ||
--
1.7.10.1.456.g16798d0

Eric Wong

unread,
May 9, 2012, 8:33:35 PM5/9/12
to mog...@googlegroups.com
These race conditions were causing this test fail occasionally.
These test failures were more common on SQLite and Postgres,
but not unheard of when using MySQL.

Some of these race conditions were due to fsck/job_master not
receiving settings changes in time, so we now resort to killing
existing processes and forcing them to reload.
---
t/60-fsck.t | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 52 insertions(+), 14 deletions(-)

diff --git a/t/60-fsck.t b/t/60-fsck.t
index 98a7bad..d79f44e 100644
--- a/t/60-fsck.t
+++ b/t/60-fsck.t
@@ -109,6 +109,24 @@ sub unblock_fsck_queue {
is($sto->retry_on_deadlock($upd), $expect, "unblocked fsck queue");
}

+sub get_worker_pids {
+ my ($admin, $worker) = @_;
+
+ ok($admin->syswrite("!jobs\n"), "requested jobs");
+ my @pids;
+
+ while (my $line = $admin->getline) {
+ last if $line =~ /^\.\r?\n/;
+
+ if ($line =~ /^$worker pids (\d[\d+\s]*)\r?\n/) {
+ @pids = split(/\s+/, $1);
+ }
+ }
+ ok(scalar(@pids), "got pid(s) of $worker");
+
+ return @pids;
+}
+
wait_for_monitor($be);

my ($req, $rv, %opts, @paths, @fsck_log, $info);
@@ -322,10 +340,18 @@ use Data::Dumper;
# stall fsck with a non-responsive host
{
is(kill("STOP", $ms1->pid), 1, "send SIGSTOP to mogstored1");
- wait_for_monitor($be);
+ wait_for_monitor($be) foreach (1..3);
+
+ foreach my $worker (qw/job_master fsck/) {
+ my (@pids) = get_worker_pids($admin, $worker);
+ is(scalar(@pids), 1, "got one $worker pid");
+ ok(kill("KILL", $pids[0]),
+ "killed $worker to reload observed down state of mogstored1");
+ }

my $delay = 10;
- ok($tmptrack->mogadm("fsck", "start"), "started fsck, sleeping ${delay}s");
+ $sto->retry_on_deadlock(sub { $sto->dbh->do("DELETE FROM file_to_queue") });
+ full_fsck($tmptrack, $dbh);
sleep $delay;
is($sto->file_queue_length(MogileFS::Config::FSCK_QUEUE), 1, "fsck queue still blocked");
}
@@ -335,6 +361,11 @@ use Data::Dumper;
is(kill("CONT", $ms1->pid), 1, "send SIGCONT to mogstored1");
wait_for_monitor($be);

+ my (@fsck_pids) = get_worker_pids($admin, "fsck");
+ is(scalar(@fsck_pids), 1, "got one fsck pid");
+ ok(kill("TERM", $fsck_pids[0]),
+ "kill fsck worker to reload observed alive state of mogstored1");
+
# force fsck to wakeup and do work again
unblock_fsck_queue($sto, 1);

@@ -370,7 +401,7 @@ use Data::Dumper;
wait_for_monitor($be);

my $delay = 10;
- ok($tmptrack->mogadm("fsck", "start"), "started fsck, sleeping ${delay}s");
+ full_fsck($tmptrack, $dbh);
sleep $delay;
is($sto->file_queue_length(MogileFS::Config::FSCK_QUEUE), 1, "fsck queue still blocked");

@@ -424,16 +455,9 @@ use Data::Dumper;
is($info->{devcount}, 2, "ensure devcount is correct at start");
wait_for_empty_queue("file_to_replicate", $dbh);

- $admin->syswrite("!jobs\n");
- my $reaper_pid;
-
- while (my $line = $admin->getline) {
- last if $line =~ /^\.\r?\n/;
-
- if ($line =~ /^reaper pids (\d+)\r?\n/) {
- $reaper_pid = $1;
- }
- }
+ my (@reaper_pids) = get_worker_pids($admin, "reaper");
+ is(scalar(@reaper_pids), 1, "only one reaper pid");
+ my $reaper_pid = $reaper_pids[0];
ok($reaper_pid, "got pid of reaper");

# reaper is watchdog is 240s, and it wakes up in 5s intervals right now
@@ -463,6 +487,8 @@ use Data::Dumper;
}

{
+ ok($tmptrack->mogadm("fsck", "stop"), "stop fsck");
+
foreach my $i (1..10) {
my $fh = $mogc->new_file("k$i", "1copy");
print $fh "$i\n";
@@ -472,8 +498,19 @@ use Data::Dumper;

ok($tmptrack->mogadm("settings", "set", "queue_rate_for_fsck", 1), "set queue_rate_for_fsck to 1");
ok($tmptrack->mogadm("settings", "set", "queue_size_for_fsck", 1), "set queue_size_for_fsck to 1");
- wait_for_monitor($be);

+ wait_for_monitor($be) foreach (1..3);
+
+ foreach my $worker (qw/job_master fsck/) {
+ my (@pids) = get_worker_pids($admin, $worker);
+ is(scalar(@pids), 1, "got one $worker pid");
+ ok(kill("KILL", $pids[0]),
+ "killed $worker to reload server settings cache");
+ }
+
+ ok($tmptrack->mogadm("fsck", "clearlog"), "clear fsck log");
+ ok($tmptrack->mogadm("fsck", "reset"), "reset fsck");
+ $sto->retry_on_deadlock(sub { $sto->dbh->do("DELETE FROM file_to_queue") });
ok($tmptrack->mogadm("fsck", "start"), "start fsck with slow queue rate");

ok(MogileFS::Config->server_setting("fsck_host"), "fsck_host set");
@@ -487,6 +524,7 @@ use Data::Dumper;
}
ok(defined($highest), "fsck_highest_fid_checked is set");
like($highest, qr/\A\d+\z/, "fsck_highest_fid_checked is a digit");
+ isnt($highest, $info->{fid}, "fsck is not running too fast");

# wait for something to get fscked
foreach my $i (1..100) {
--
1.7.10.1.456.g16798d0

Eric Wong

unread,
May 11, 2012, 11:19:10 PM5/11/12
to mog...@googlegroups.com
Two more changes pushed out:

fsck: update devcount, forget devs on unfixable FIDs
fsck: cleanup and reduce unnecessary devcount updates

The first is a noticeable change in it changes stats reporting for
unfixable FIDs. The second should reduce DB load (haven't tested
performance impact) and relies on another commit[1] from this branch
ensure everything works.

[1] - commit eba5cadddd8fddcb2b1f21547202cae4501fdbaa
fsck: log bad count correctly instead of policy violation

Tests pass on all databases we support.

lib/MogileFS/Worker/Fsck.pm | 52 ++++++++++++++++++++++++++-------------------
t/60-fsck.t | 3 +--
2 files changed, 31 insertions(+), 24 deletions(-)

$ git pull git://bogomips.org/MogileFS-Server fsck-tests

I should probably rename this branch to just "fsck", since I'm
now changing (and hopefully improving) fsck.

Eric Wong

unread,
May 11, 2012, 11:19:56 PM5/11/12
to mog...@googlegroups.com
Whenever an FID is unfixable, be sure to update devcount (to
zero) to easily inform the user via mogstats. If the FID
magically reappears in the future, the desperate fallback mode
will still find the file.
---
lib/MogileFS/Worker/Fsck.pm | 22 +++++++++++++++-------
t/60-fsck.t | 3 +--
2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/lib/MogileFS/Worker/Fsck.pm b/lib/MogileFS/Worker/Fsck.pm
index 0e82977..36ca475 100644
--- a/lib/MogileFS/Worker/Fsck.pm
+++ b/lib/MogileFS/Worker/Fsck.pm
@@ -303,7 +303,11 @@ sub fix_fid {
$check_dfids->("desperate");

# still can't fix it?
- return CANT_FIX unless @good_devs;
+ unless (@good_devs) {
+ $self->forget_bad_devs($fid, @bad_devs);
+ $fid->update_devcount;
+ return CANT_FIX;
+ }

# wow, we actually found it!
$fid->fsck_log(EV_FOUND_FID);
@@ -313,12 +317,7 @@ sub fix_fid {
# wrong, with only one file_on record...) and re-replicate
}

- # remove the file_on mappings for devices that were bogus/missing.
- foreach my $bdev (@bad_devs) {
- error("removing file_on mapping for fid=" . $fid->id . ", dev=" . $bdev->id);
- $fid->forget_about_device($bdev);
- }
-
+ $self->forget_bad_devs($fid, @bad_devs);
# in case the devcount or similar was fixed.
$fid->want_reload;

@@ -450,6 +449,15 @@ sub fix_checksums {
}
}

+# remove the file_on mappings for devices that were bogus/missing.
+sub forget_bad_devs {
+ my ($self, $fid, @bad_devs) = @_;
+ foreach my $bdev (@bad_devs) {
+ error("removing file_on mapping for fid=" . $fid->id . ", dev=" . $bdev->id);
+ $fid->forget_about_device($bdev);
+ }
+}
+
1;

# Local Variables:
diff --git a/t/60-fsck.t b/t/60-fsck.t
index d79f44e..95dfefc 100644
--- a/t/60-fsck.t
+++ b/t/60-fsck.t
@@ -319,8 +319,7 @@ use Data::Dumper;
wait_for_empty_queue("file_to_queue", $dbh);
$info = $mogc->file_info($key);

- # XXX devcount probably needs to be updated on GONE
- # is($info->{devcount}, 2, "devcount updated to zero");
+ is($info->{devcount}, 0, "devcount updated to zero");
@paths = $mogc->get_paths($key);
is(scalar(@paths), 0, "get_paths returns nothing");
}
--
1.7.10.1.456.g16798d0

Eric Wong

unread,
May 11, 2012, 11:20:14 PM5/11/12
to mog...@googlegroups.com
fix_fid(): we no longer rely blindly update devcount on every
call. This is important because we call fix_fid() on checksum
checks regardless, and devcount updates entail unnecessary
updates to the `file' table.

While we're at it, consolidate the places where we check the
skip_devcount flag and log bad devcounts.
---
lib/MogileFS/Worker/Fsck.pm | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/lib/MogileFS/Worker/Fsck.pm b/lib/MogileFS/Worker/Fsck.pm
index 36ca475..82afbc6 100644
--- a/lib/MogileFS/Worker/Fsck.pm
+++ b/lib/MogileFS/Worker/Fsck.pm
@@ -137,14 +137,9 @@ sub check_fid {
}

# This is a simple fixup case
- unless (MogileFS::Config->server_setting_cached('skip_devcount') || scalar($fid->devids) == $fid->devcount) {
- # log a bad count
- $fid->fsck_log(EV_BAD_COUNT);
-
- # TODO: We could fix this without a complete fix pass
- # $fid->update_devcount();
- return $fix->();
- }
+ # If we got here, we already know we have no policy violation and
+ # don't need to call $fix->() to just fix a devcount
+ $self->maybe_fix_devcount($fid);

# missing checksum row
if ($fid->class->hashtype && ! $fid->checksum) {
@@ -217,9 +212,6 @@ sub fix_fid {
my ($self, $fid) = @_;
debug(sprintf("Fixing FID %d", $fid->id));

- # This should happen first, since the fid gets awkwardly reloaded...
- $fid->update_devcount;
-
# make devfid objects from the devids that this fid is on,
my @dfids = map { MogileFS::DevFID->new($_, $fid) } $fid->devids;

@@ -332,10 +324,7 @@ sub fix_fid {
}

# Clean up the device count if it's wrong
- unless(MogileFS::Config->server_setting_cached('skip_devcount') || scalar($fid->devids) == $fid->devcount) {
- $fid->update_devcount();
- $fid->fsck_log(EV_BAD_COUNT);
- }
+ $self->maybe_fix_devcount($fid);

return HANDLED;
}
@@ -458,6 +447,17 @@ sub forget_bad_devs {
}
}

+sub maybe_fix_devcount {
+ # don't even log BCNT errors if skip_devcount is enabled
+ return if MogileFS::Config->server_setting_cached('skip_devcount');
+
+ my ($self, $fid) = @_;
+ return if scalar($fid->devids) == $fid->devcount;
+ # log a bad count
+ $fid->fsck_log(EV_BAD_COUNT);
+ $fid->update_devcount();
+}
+
1;

# Local Variables:
--
1.7.10.1.456.g16798d0

dormando

unread,
May 15, 2012, 8:15:48 PM5/15/12
to mog...@googlegroups.com
This seems heavy handed... I don't have any alternatives offhand though.

Seems like restarting the processes could potentially hide bugs coming
from a combination of state changes.

Eric Wong

unread,
May 16, 2012, 3:35:09 PM5/16/12
to mog...@googlegroups.com
dormando <dorm...@rydia.net> wrote:
> This seems heavy handed... I don't have any alternatives offhand though.

I could add more long sleeps in the test based on what we know about
every(). This test already sleeps too much, and I'm not sure what
to do about it, perhaps make !watch broadcast when workers are bored.

> Seems like restarting the processes could potentially hide bugs coming
> from a combination of state changes.

I think it's fine since the newly forked children will inherit from the
ProcManager. I'll double check the state change stuff.

dormando

unread,
May 16, 2012, 3:39:06 PM5/16/12
to mog...@googlegroups.com
> dormando <dorm...@rydia.net> wrote:
> > This seems heavy handed... I don't have any alternatives offhand though.
>
> I could add more long sleeps in the test based on what we know about
> every(). This test already sleeps too much, and I'm not sure what
> to do about it, perhaps make !watch broadcast when workers are bored.

Some debug or test mode to "reflect" a ":updated_state" back up to
ProcManager after receiving a new state dump? Then you'd know when
JobMaster got an update or whatnot.

> > Seems like restarting the processes could potentially hide bugs coming
> > from a combination of state changes.
>
> I think it's fine since the newly forked children will inherit from the
> ProcManager. I'll double check the state change stuff.

It's more like future proofing. though I tried hard to make JobMaster
pretty stateless, it's been jammed a few times before.

Eric Wong

unread,
May 17, 2012, 5:53:11 AM5/17/12
to mog...@googlegroups.com
dormando <dorm...@rydia.net> wrote:
> > dormando <dorm...@rydia.net> wrote:
> > > This seems heavy handed... I don't have any alternatives offhand though.
> >
> > I could add more long sleeps in the test based on what we know about
> > every(). This test already sleeps too much, and I'm not sure what
> > to do about it, perhaps make !watch broadcast when workers are bored.
>
> Some debug or test mode to "reflect" a ":updated_state" back up to
> ProcManager after receiving a new state dump? Then you'd know when
> JobMaster got an update or whatnot.

I started adding a !debug command, but ultimately just went with
"!to $worker :shutdown" for now (patches coming).

But maybe !debug is worth adding anyways?

diff --git a/lib/MogileFS/Connection/Client.pm b/lib/MogileFS/Connection/Client.pm
index bf58222..c2f1351 100644
--- a/lib/MogileFS/Connection/Client.pm
+++ b/lib/MogileFS/Connection/Client.pm
@@ -131,7 +131,11 @@ sub handle_admin_command {
} elsif ($cmd =~ /^recent/) {
# show the most recent N queries
push @out, MogileFS::ProcManager->RecentQueries;
-
+ } elsif ($cmd =~ /^debug/) {
+ if (defined $args && $args =~ /(\d+)/) {
+ $Mgd::DEBUG = $1;
+ }
+ push @out, "debug level $Mgd::DEBUG";
} elsif ($cmd =~ /^version/) {
# show the most recent N queries
push @out, $MogileFS::Server::VERSION;
diff --git a/lib/MogileFS/Util.pm b/lib/MogileFS/Util.pm
index 42d23f8..a67aba7 100644
--- a/lib/MogileFS/Util.pm
+++ b/lib/MogileFS/Util.pm
@@ -54,6 +54,7 @@ sub apply_state_events {
$factories{$type}->remove($old) if $old;
}
}
+ debug("applied ${$_[0]}");
}

sub every {

> > > Seems like restarting the processes could potentially hide bugs coming
> > > from a combination of state changes.
> >
> > I think it's fine since the newly forked children will inherit from the
> > ProcManager. I'll double check the state change stuff.
>
> It's more like future proofing. though I tried hard to make JobMaster
> pretty stateless, it's been jammed a few times before.

I feel the state stuff has gotten to the point where it'd be hard to
break again :) Anyways, Mogile's been good about recovering from
occasionally dying processes in my experience, and with the graceful
nature of "!to ... :shutdown" there should be even less to worry about.

dormando

unread,
May 17, 2012, 3:28:15 PM5/17/12
to mog...@googlegroups.com
> I started adding a !debug command, but ultimately just went with
> "!to $worker :shutdown" for now (patches coming).

That is *way* better!

> But maybe !debug is worth adding anyways?

http://code.google.com/p/mogilefs/issues/detail?id=10 ? I never finished
verifying how to enable/disable debug at runtime. It seemed like there
*was* a way. but I left the bug open, since being able to enable/disable
it globally *and* per-worker at runtime would be great to have.

> I feel the state stuff has gotten to the point where it'd be hard to
> break again :) Anyways, Mogile's been good about recovering from
> occasionally dying processes in my experience, and with the graceful
> nature of "!to ... :shutdown" there should be even less to worry about.

Where there's a dormando, there's a bug! Remember my deadlock issue which
caused JobMaster to hang until you killed it manually or restarted the
tracker? :P
Reply all
Reply to author
Forward
0 new messages