raid1 error handling and faulty drives

I've been looking at ways to minimize the impact of a faulty drive in a raid1 array. Our major problem is that a faulty drive can absorb lots of wall clock time in error recovery within the device driver (SATA libata error handler in this case), during which any further raid activity is blocked and the system effectively hangs. This tends to negate the high availability advantage of placing the file system on a RAID array in the first place.

We've had one particularly bad drive, for example, which could sync without indicating any write errors but as soon as it became active in the array would start yielding read errors. It this particular case it would take 30 minutes or more for the process to progress to a point where some fatal error would occur to kick the drive out of the array and return the system to normal opreation.

For SATA, this effect can be partially mitigated by reducing the default 30 second timeout at the SCSI layer (/sys/block/sda/device/timeout). However, the system stills spends 45 seconds or so per retry in the driver issuing various reset operations in an attempt to recover from the error before returning control to the SCSI layer.

I've been experimenting with a patch which makes two basic changes.

1) It issues the first read request against a mirror with more than 1 drive active using the BIO_RW_FAILFAST flag to short-circuit the SCSI layer from re-trying the failed operation in the low level device driver the default 5 times.

2) It adds a threshold on the level of recent error acivity which is acceptable in a given interval, all configured through /sys. If a mirror has generated more errors in this interval than the threshold, it is kicked out of the array.

One would think that #2 should not be necessary as the raid1 retry logic already attempts to rewrite and then reread bad sectors and fails the drive if it cannot do both. However, what we observe is that the re-write step succeeds as does the re-read but the drive is really no more healthy. Maybe the re-read is not actually going out to the media in this case due to some caching effect?

This patch (against 2.6.20) still contains some debugging printk's but should be otherwise functional. I'd be interested in any feedback on this specific approach and would also be happy if this served to foster an error recovery discussion which came up with some even better approach.

Mike Accetta

ECI Telecom Ltd.
Transport Networking Division, US (previously Laurel Networks)
---

diff -Naurp 2.6.20/drivers/md/md.c kernel/drivers/md/md.c
@@ -1842,6 +1842,46 @@ static struct rdev_sysfs_entry rdev_erro
 __ATTR(errors, S_IRUGO|S_IWUSR, errors_show, errors_store);
 
 static ssize_t
+errors_threshold_show(mdk_rdev_t *rdev, char *page)
+{
+	return sprintf(page, "%d\n", rdev->errors_threshold);
+}
+
+static ssize_t
+errors_threshold_store(mdk_rdev_t *rdev, const char *buf, size_t len)
+{
+	char *e;
+	unsigned long n = simple_strtoul(buf, &e, 10);
+	if (*buf && (*e == 0 || *e == '\n')) {
+		rdev->errors_threshold = n;
+		return len;
+	}
+	return -EINVAL;
+}
+static struct rdev_sysfs_entry rdev_errors_threshold =
+__ATTR(errors_threshold, S_IRUGO|S_IWUSR, errors_threshold_show, errors_threshold_store);
+
+static ssize_t
+errors_interval_show(mdk_rdev_t *rdev, char *page)
+{
+	return sprintf(page, "%d\n", rdev->errors_interval);
+}
+
+static ssize_t
+errors_interval_store(mdk_rdev_t *rdev, const char *buf, size_t len)
+{
+	char *e;
+	unsigned long n = simple_strtoul(buf, &e, 10);
+	if (*buf && (*e == 0 || *e == '\n')) {
+		rdev->errors_interval = n;
+		return len;
+	}
+	return -EINVAL;
+}
+static struct rdev_sysfs_entry rdev_errors_interval =
+__ATTR(errors_interval, S_IRUGO|S_IWUSR, errors_interval_show, errors_interval_store);
+
+static ssize_t
 slot_show(mdk_rdev_t *rdev, char *page)
 {
 	if (rdev->raid_disk < 0)

@@ -1925,6 +1965,8 @@ static struct attribute *rdev_default_at
 	&rdev_state.attr,
 	&rdev_super.attr,
 	&rdev_errors.attr,
+	&rdev_errors_interval.attr,
+	&rdev_errors_threshold.attr,
 	&rdev_slot.attr,
 	&rdev_offset.attr,
 	&rdev_size.attr,

@@ -2010,6 +2052,9 @@ static mdk_rdev_t *md_import_device(dev_
 	rdev->flags = 0;
 	rdev->data_offset = 0;
 	rdev->sb_events = 0;
+	rdev->errors_interval = 15;	/* minutes */
+	rdev->errors_threshold = 16;	/* sectors */
+	rdev->errors_asof = INITIAL_JIFFIES;
 	atomic_set(&rdev->nr_pending, 0);
 	atomic_set(&rdev->read_errors, 0);
 	atomic_set(&rdev->corrected_errors, 0);

diff -Naurp 2.6.20/include/linux/raid/md_k.h kernel/include/linux/raid/md_k.h
@@ -104,6 +104,12 @@ struct mdk_rdev_s
 					   * for reporting to userspace and storing
 					   * in superblock.
 					   */
+	u64		errors_asof;	/* begin time of current read errors interval */
+	int		errors_interval;/* duration of the read errors interval */
+	int		errors_threshold;/* maximum read errors allowed in any
+					  * interval before we will raise an
+					  * array error
+					  */
 };
 
 struct mddev_s

I've recently become aware that we really need FAILFAST - possibly for all IO from RAID1/5. Modern drives don't need any retry at the OS level - if the retry in the firmware cannot get the data, nothing will.

This is probably a good idea. It bothers me a little to require 2 separate numbers in sysfs...

When we get a read error, we quiesce the device, the try to sort out the read errors, so we effectively handle them in batches. Maybe we should just set a number of seconds, and if there are a 3 or more batches in that number of seconds, we kick the drive... just a thought.

I have occasionally wondered if a cache would defeat this test. I wonder if we can push a "FORCE MEDIA ACCESS" flag down with that read. I'll ask.

Thanks. I agree that we do need something along these lines. It might be a while before I can give the patch the brainspace it deserves as I am travelling this fortnight.

NeilBrown

I think I was just trying to be as flexible as possible. If we were to use one number, I'd do the opposite and fix the interval but allow the threshold to be configured just because I would tend to think about a disk being bad in terms of it having a "more than" an expected number of errors in some fixed interval rather than because it had a fixed number of errors in "less than" some expected interval. Mathematically the approaches ought to be equivalent.

I looked around for something like this but it doesn't appear to be implemented that I could see. I couldn't even find an explicit mention of read caching in any drive specs to begin with. Read-ahead seems to be the closest concept.

Looking forward to further discussion. Thank you! --
Mike Accetta

ECI Telecom Ltd.
Transport Networking Division, US (previously Laurel Networks)

Neil,

Was anything ever done with this idea? I can throw my hat into the this-is-a-big-problem ring. I oftentimes have a RAID1 disk fail on a heavy-I/O system and the system basically needs to be power-cycled if it is to come back up within the hour (any OS access to that RAID will hang).

Any word would be appreciated.

Philip