Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#4179 closed defect (fixed)

(patch included) BackendQueryDiskSpace incorrectly aggregates identical local disks

Reported by: cmoates@… Owned by: Janne Grunau
Priority: minor Milestone: 0.21
Component: mythtv Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

I just installed three brand new, identical disks, partitioned them, and formatted them. They have the same capacity and free space.

When going into System Status, I noticed the following:

MythTV Drive #0:
  Directories:
    /mnt/myth/sda1
    /mnt/myth/sdb1
    /mnt/myth/sdc1

It thinks they are all the same disk, because of the method used to determine if two disks are "the same disk but in different places."

Looking at the source, I noticed the following test being made:

if (total capacity of two disks is about the same) && (total free space of two disks is about the same) then (they are the same disk)

I've changed this to say:

if (total capacity of two disks is about the same) && (total free space of two disks is about the same) && (NOT (both disks are local disks)) then (they are the same disk)

This resolves the issue of two identical capacity drives with roughly identical free space being aggregated.

Note that this affected the "amount of total disk space available for myth" calculations as well.

Here's the patch as generated by svn diff:

Index: programs/mythbackend/backendutil.cpp
===================================================================
--- programs/mythbackend/backendutil.cpp        (revision 14666)
+++ programs/mythbackend/backendutil.cpp        (working copy)
@@ -178,7 +178,8 @@
             // different than when it is locally mounted because of block sizes
             if ((absLongLong(it1->totalSpaceKB - it2->totalSpaceKB) <= 16 ) &&
                 (absLongLong(it1->usedSpaceKB - it2->usedSpaceKB)
-                 < maxWriteFiveSec))
+                 < maxWriteFiveSec) &&
+                (!(it1->isLocal && it2->isLocal)))
             {
                 if (!it1->hostname.contains(it2->hostname))
                     it1->hostname = it1->hostname + "," + it2->hostname;

Thanks,

Chris Moates

Attachments (1)

mythdiskspacebug.patch (2.0 KB) - added by Chris Moates <cmoates@…> 12 years ago.
Patch for identical disk space edge case using stat()

Download all attachments as: .zip

Change History (7)

comment:1 Changed 12 years ago by Chris Moates <cmoates@…>

I failed to take into account two directories on the same disk, for different storage groups. As a result, this patch will fix one bug but introduce another. Please reject this patch.

comment:2 Changed 12 years ago by cpinkham

Owner: changed from Isaac Richards to cpinkham

comment:3 Changed 12 years ago by Janne Grunau

Owner: changed from cpinkham to Janne Grunau

I have patches in my queue which will at least help to avoid this situation.

It should be safe to count only the bitrate of busy recorders in the maxWriteFiveSec calculation.

The second improvement comes from the autoexpirer rewrite. The increased number of recorders caused by multirec develepment needs a smarter auto expirer which expires only on file systems we use for recording with the rate of the busy recorders for this file system.

My current implemtation needs stable fsID and will not join file systems it once identified as distinct.

comment:4 Changed 12 years ago by Chris Moates <cmoates@…>

I'm going to attach my second attempt to fix this bug. I've heavily commented the code both to make sure my logic isn't flawed and to ensure that it's understood why I'm doing what I'm doing.

The patch will only act when both disks are local, and then attempt to check the st_dev device ID from stat(). If they match, it will consider them to be separate disks. Under any other situation, the original calculation stands, and no change will be made. I've tested this with local and NFS mounted disk, and it seems to function as expected.

It sounds like your other patches may be able to supercede this one, or perhaps they can strengthen each other.

Changed 12 years ago by Chris Moates <cmoates@…>

Attachment: mythdiskspacebug.patch added

Patch for identical disk space edge case using stat()

comment:5 Changed 12 years ago by Janne Grunau

Resolution: fixed
Status: newclosed

(In [14894]) Closes #4179. Use struct stat.dev to determine if local recording dirs are on distinct file systems

based on a patch by <Chris Moates cmoates {a} gmail {d} com>

comment:6 Changed 12 years ago by Janne Grunau

(In [14922]) Refs #4179, #4197. reverts [14894]. In the current form to buggy

stat calls are only done on the master backend but is isLocal is filled by each backend I hope that [1497] and [14899] solves the problem too.

Note: See TracTickets for help on using tickets.