[#LU-427] Test failure on test suite lfsck

[LU-427] Test failure on test suite lfsck Created: 17/Jun/11 Updated: 09/May/12 Resolved: 16/Feb/12
Status:	Resolved
Project:	Lustre
Component/s:	None
Affects Version/s:	Lustre 2.2.0, Lustre 2.1.1
Fix Version/s:	Lustre 2.2.0, Lustre 2.1.2, Lustre 1.8.8

Type:

Bug

Priority:

Blocker

Reporter:

Maloo

Assignee:

Yang Sheng

Resolution:

Fixed

Votes:

Labels:

None

Attachments:

lu427.tar.gz

mdsdb

Issue Links:

Duplicate
is duplicated by	~~LU-999~~	Test failure on test suite lfsck	Resolved

Severity:

Rank (Obsolete):

4722

Description

This issue was created by maloo for sarah <sarah@whamcloud.com>

This issue relates to the following test suite run: https://maloo.whamcloud.com/test_sets/e58b60d2-98ae-11e0-9a27-52540025f9af.
lfsck failed with quota enabled on the latest rhel5/i686 build #176

Comments

Comment by Peter Jones [ 17/Jun/11 ]

Yang Sheng will look into this one

Comment by Peter Jones [ 25/Jun/11 ]

Yang Sheng

Any progress to report? This came up during 2.1 testing so we would like to understand it.

Thanks

Peter

Comment by Yang Sheng [ 27/Jun/11 ]

Hi, Peter, I haven't found any obvious error from the log files. It looks like hard to reproduce.

Comment by Peter Jones [ 13/Jul/11 ]

Sarah

You have hit this same error (reported as LU439) for the last two 2.1 builds. Are you able to provide some more information to enable YangSheng to move forward on fixing it?

Peter

Comment by Sarah Liu [ 13/Jul/11 ]

Hi YangSheng,

This bug can be reproduce on both the latest rhel5 and rhel6 x86_64(build #201) with quota enabled. please see the attached for more logs. If you need anything else, just let me know.

Comment by Yang Sheng [ 13/Jul/11 ]

Many Thanks for the information. Sarah, I'll trying to repduce it on VM.

Comment by Yang Sheng [ 15/Jul/11 ]

Hi, Sarah, Could you please attached '/tmp/mdsdb' to here if you hit it again. TIA.

Comment by Sarah Liu [ 18/Jul/11 ]

please find mdsdb in the attached.

Comment by Yang Sheng [ 19/Jul/11 ]

I don't find any wrong in this mdsdb file.

VERSION=3
format=bytevalue
database=mdshdr
type=hash
db_pagesize=512
HEADER=END
01cdabdb00000000
01cdabdb000000000100000000000000ea0000000000000006000000000000001b000000000000001b000000000000001b000000000000001b000000000000001c000000000000001
c0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
000000000000000000000000000000000000000000000

and this file can be used for manual check:

[root@localhost git]# e2fsck -n --mdsdb mdsdb --ostdb odb /tmp/lustre-ost1
e2fsck 1.41.90.wc3 (28-May-2011)
lustre-OST0000 has been mounted 79 times without being checked, check forced.
Pass 1: Checking inodes, blocks, and sizes
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
Pass 6: Acquiring OST information for lfsck
lustre-OST0000: 125/50016 files (3.2% non-contiguous), 18088/50000 blocks
[root@localhost git]#

Comment by Yang Sheng [ 27/Jul/11 ]

This is a test environment issue. The /tmp should be a shared directory between the test nodes, but it isn't. I'll do further investigate for that. Thanks Sarah provide the test nodes.

Comment by Yang Sheng [ 27/Jul/11 ]

Hi, Sarah, You can define SHARED_DIRECTORY to a shared directory to avoid failed on lfsck test. And this isn't a regression i think.

Comment by Sarah Liu [ 27/Jul/11 ]

Hi Yang Sheng, /scratch on Toro is the shared directory which I use for all tests for a long time. I think the script should have the flexibility to use not only /tmp as a shared dir.

[root@fat-intel-4 scratch]# touch aaa
[root@fat-intel-4 scratch]# ls
186-tempfile.rb hudson rhel6-64.xml
187-tempfile.rb ISO rhel6-i386_build01.xml
aaa ksrc rhel6-x86_64_build01.xml
client-11.rpms logs src
client-15.rpms lost+found tests
coreutils-6.12-32.24.1.src.rpm repos
ftp rhel6-32.xml
client-5:~ # ls /scratch/
186-tempfile.rb ftp rhel6-64.xml
187-tempfile.rb hudson rhel6-i386_build01.xml
ISO ksrc rhel6-x86_64_build01.xml
aaa logs src
client-11.rpms lost+found tests
client-15.rpms repos
coreutils-6.12-32.24.1.src.rpm rhel6-32.xml

Comment by Yang Sheng [ 02/Aug/11 ]

The main problem is that lfsck has invoked the generate_db() to check whether a directory is shared or not. It should stop when /tmp isn't a shared.

Comment by Sarah Liu [ 02/Aug/11 ]

Hi Yang Sheng,

I think the problem is in lfsck.sh, it invokes init_test_env before reads the configuration file. In init_test_env, it exports SHARED_DIRECTORY, MDSDB and OSTDB as default values(/tmp, /tmp/mdsdb and /tmp/ostdb), while in config file, I explicit /scratch as the SHARED_DIRECTORY. That is the reason why generate_db() doesn't quit since at that time, SHARED_DIRECTORY is set to the correct one, but MDSDB and OSTDB still have the "wrong" values.

Comment by Yang Sheng [ 04/Aug/11 ]

patch upload to: http://review.whamcloud.com/1180

Comment by Jian Yu [ 09/Aug/11 ]

If LFSCK_ALWAYS="yes", then lfsck would be run after each test suite which calls check_and_cleanup_lustre(). So, re-defining MDSDB and OSTDB only in lfsck.sh is not enough. I think maybe we could move the definitions of the following variables from init_test_env() to cfg/local.sh:

    # This is used by a small number of tests to share state between the client
    # running the tests, or in some cases between the servers (e.g. lfsck.sh).
    # It needs to be a non-lustre filesystem that is available on all the nodes.
    export SHARED_DIRECTORY=${SHARED_DIRECTORY:-"/tmp"}
    export MDSDB=${MDSDB:-$SHARED_DIRECTORY/mdsdb}
    export OSTDB=${OSTDB:-$SHARED_DIRECTORY/ostdb}

And remove the following lines from cfg/ncli.sh since it sources cfg/local.sh:

# This is used by a small number of tests to share state between the client
# running the tests, or in some cases between the servers (e.g. lfsck.sh).
# It needs to be a non-lustre filesystem that is available on all the nodes.
SHARED_DIRECTORY=${SHARED_DIRECTORY:-""}        # bug 17839 comment 65

We also need add a function to verify that the $SHARED_DIRECTORY is a real shared directory among the test nodes in the Lustre cluster and
replace those codes (lfsck.sh, recovery-*-scale.sh, etc.) which simply check the variable of $SHARED_DIRECTORY is empty or not.

What do you think of these changes, Sarah and Yang Sheng?

Comment by Yang Sheng [ 09/Aug/11 ]

If LFSCK_ALWAYS="yes", then lfsck would be run after each test suite which calls check_and_cleanup_lustre(). So, re-defining MDSDB and OSTDB only in lfsck.sh is not enough. I think maybe we could move the definitions of the following variables from init_test_env() to cfg/local.sh:

    # This is used by a small number of tests to share state between the client
    # running the tests, or in some cases between the servers (e.g. lfsck.sh).
    # It needs to be a non-lustre filesystem that is available on all the nodes.
    export SHARED_DIRECTORY=${SHARED_DIRECTORY:-"/tmp"}
    export MDSDB=${MDSDB:-$SHARED_DIRECTORY/mdsdb}
    export OSTDB=${OSTDB:-$SHARED_DIRECTORY/ostdb}

And remove the following lines from cfg/ncli.sh since it sources cfg/local.sh:

# This is used by a small number of tests to share state between the client
# running the tests, or in some cases between the servers (e.g. lfsck.sh).
# It needs to be a non-lustre filesystem that is available on all the nodes.
SHARED_DIRECTORY=${SHARED_DIRECTORY:-""}        # bug 17839 comment 65

If define MDSDB & OSTDB in cfg.sh then we needn't any change for it. I have not objection.

We also need add a function to verify that the $SHARED_DIRECTORY is a real shared directory among the test nodes in the Lustre cluster and
replace those codes (lfsck.sh, recovery-*-scale.sh, etc.) which simply check the variable of $SHARED_DIRECTORY is empty or not.

Looks like generate_db() already able to do this check.

Comment by Sarah Liu [ 09/Aug/11 ]

Is that workable that just changing the order of init_test_env and the call of configuration file? I mean, read in the config file first and then call the init_test_env

Comment by Yang Sheng [ 09/Aug/11 ]

Is that workable that just changing the order of init_test_env and the call of configuration file? I mean, read in the config file first and then call the init_test_env

Of course it works. But all the scripts called in this order. Seem read config file first and then call init_test_env more reasonable. I wonder who know the original intend for this order?

Comment by Jian Yu [ 10/Aug/11 ]

Looks like generate_db() already able to do this check.

It seemed that the function did not detect that /tmp was not a shared directory among the test nodes. We still need a common function to do the check since $SHARED_DIRECTORY is not only used for storing $MDSDB and $OSTDB, it's also used by recovery-*-scale.sh tests to catch failed clients.

Of course it works. But all the scripts called in this order. Seem read config file first and then call init_test_env more reasonable. I wonder who know the original intend for this order?

I've no idea about this. After looking into init_test_env() and cfg/

{local,ncli}

.sh, I found it seemed that only the following two stuff would be affected if we ran init_test_env after sourcing the config file:
1) L_GETIDENTITY
Currently, this variable is defined in init_test_env() and used in cfg/local.sh.
2) HOSTNAME
Currently, this variable is defined in init_test_env() and used in init_clients_lists in cfg/ncli.sh.

So, if we'd prefer to change the order, we need define those variables in cfg/local.sh and do more testing to see whether there are other changes need to be done.

Comment by Andreas Dilger [ 07/Feb/12 ]

My memory of the last time I looked at switching the init_test_env() and the config file, I was in favor of switching it. This would allow the config file to drop the setting of default values, and only have to specify values that it wants to change from the default. Then init_test_env() can supply all of the unset values, and use those for setting its own defaults. In fact, it would be even better if init_test_env() handled reading the config file, or that the config file called init_test_env() itself to set the defaults. Reducing the per-script initialization to a single line would reduce the complexity and duplication in all of the tests. I see something similar to the following lines copied to the start of each test:

LUSTRE=${LUSTRE:-`dirname $0`/..}
. $LUSTRE/tests/test-framework.sh
init_test_env $@
. ${CONFIG:=$LUSTRE/tests/cfg/$NAME.sh}
init_logging

That said, I'd be happier to have the lfsck.sh test failure fixed quickly in a simple patch, and then a separate patch which is only switching the order of the test initialization. That reduces the risk and complexity of the fix for the failing lfsck.sh test, and also ensures that tests are passing sooner instead of later and avoids blocking other patches from landing for a longer time.

Comment by Jian Yu [ 10/Feb/12 ]

Lustre Tag: v2_1_1_0_RC1
Distro/Arch: RHEL5.7 (server), SLES11SP1 (client)
Lustre Build: http://build.whamcloud.com/job/lustre-b2_1/37/
Network: TCP

The same issue occurred:
https://maloo.whamcloud.com/test_sets/5c26439a-5300-11e1-81a2-5254004bbbd3

Comment by Peter Jones [ 16/Feb/12 ]

Landed for 2.2