Moodle

Legacy file sesssions: when disk is full, login is impossible but no errors are reported

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9.5, 2.0
  • Fix Version/s: 1.9.6, 2.0
  • Component/s: Authentication
  • Labels:
    None
  • Difficulty:
    Easy
  • Affected Branches:
    MOODLE_19_STABLE, MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE, MOODLE_20_STABLE

Description

When using the legacy file sessions on a partition which is full, logging in is impossible (you get returned to the homepage unauthenticated) but there are no errors message reported anywhere.

The attach patch (against HEAD) adds a new check when initialising the session directory and prints out a helpful message.

Attachments

  1. Text File
    freespace.patch
    20/May/09 12:07 PM
    0.8 kB
    Francois Marier
  2. Text File
    sessiondiskfull.patch
    18/May/09 12:46 PM
    2 kB
    Francois Marier

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Jeffrey Silverman added a comment -

This patch breaks on large filesystems. Or at least, on our large (>6TB) filesystem.

Example (the important part is the PHP warning):

/clients/mr195preview/sessions<br>1.<br>PHP Warning: disk_free_space(): Value too large for defined data type in /mroomstech/df.php on line 10

I would have thought that 6TB is okay for a var type "double" so I'm not sure why this fails, but, alas, it does. We hate to have to patch this locally, so if someone could take a closer look, that would be great.

I'll report back if I have more detail.

Thanks!

Show
Jeffrey Silverman added a comment - This patch breaks on large filesystems. Or at least, on our large (>6TB) filesystem. Example (the important part is the PHP warning): /clients/mr195preview/sessions<br>1.<br>PHP Warning: disk_free_space(): Value too large for defined data type in /mroomstech/df.php on line 10 I would have thought that 6TB is okay for a var type "double" so I'm not sure why this fails, but, alas, it does. We hate to have to patch this locally, so if someone could take a closer look, that would be great. I'll report back if I have more detail. Thanks!
Hide
Permalink
Petr Škoda (skodak) added a comment -

reopening, please fix ASAP

Show
Petr Škoda (skodak) added a comment - reopening, please fix ASAP
Hide
Permalink
Francois Marier added a comment -

Hi Jeffrey,

I don't have a large enough partition to be able to reproduce your problem, so could you tell me:

1- what the return value of "disk_free_space()" is?

i.e. doing something like: var_dump(disk_free_space($CFG->dataroot.'/sessions'));

2- does disabling debugging before the test and restoring it afterwards make the errors go away?

Depending on how this works, I'm thinking that we could disable error checking during the call to disk_free_space and if it returns a NaN, then we could assume that the disk isn't full...

Otherwise, I guess there will be no choice but to revert this check, which would suck.

Cheers,
Francois

Show
Francois Marier added a comment - Hi Jeffrey, I don't have a large enough partition to be able to reproduce your problem, so could you tell me: 1- what the return value of "disk_free_space()" is? i.e. doing something like: var_dump(disk_free_space($CFG->dataroot.'/sessions')); 2- does disabling debugging before the test and restoring it afterwards make the errors go away? Depending on how this works, I'm thinking that we could disable error checking during the call to disk_free_space and if it returns a NaN, then we could assume that the disk isn't full... Otherwise, I guess there will be no choice but to revert this check, which would suck. Cheers, Francois
Hide
Permalink
Jeffrey Silverman added a comment -

This may be related to this PHP bug:

http://bugs.php.net/bug.php?id=27792

Unfortunately I cannot confirm yet. If that is the case, then it isn't really a Moodle bug; I will post back when I confirm.

Also, here is the result of the requested var_dump() call.

Script to produce is as follows:
<?php
var_dump(disk_free_space('/clients/demomroomstest/sessions'));
?>

Output is:

PHP Warning: disk_free_space(): Value too large for defined data type in /www/df2.php on line 2
bool(false)

And, just to make sure it is working on smaller filesystems, I changed the var_dump as follows:
<?php
var_dump(disk_free_space('/var/tmp'));
?>

Which outputs:

float(58894353408)

Thanks!

Show
Jeffrey Silverman added a comment - This may be related to this PHP bug: http://bugs.php.net/bug.php?id=27792 Unfortunately I cannot confirm yet. If that is the case, then it isn't really a Moodle bug; I will post back when I confirm. Also, here is the result of the requested var_dump() call. Script to produce is as follows: <?php var_dump(disk_free_space('/clients/demomroomstest/sessions')); ?> Output is: PHP Warning: disk_free_space(): Value too large for defined data type in /www/df2.php on line 2 bool(false) And, just to make sure it is working on smaller filesystems, I changed the var_dump as follows: <?php var_dump(disk_free_space('/var/tmp')); ?> Which outputs: float(58894353408) Thanks!
Hide
Permalink
Francois Marier added a comment -

Hi Jeffrey,

Could you test this patch (against 1.9) to see if it succesfully works around the problem?

Cheers,
Francois

Show
Francois Marier added a comment - Hi Jeffrey, Could you test this patch (against 1.9) to see if it succesfully works around the problem? Cheers, Francois
Hide
Permalink
Jeffrey Silverman added a comment -

Howdy. The patch seems reasonable, (as long as disk_free_space() returns false on fail, obviously).

I'll try a test and report back.

Also, I was thinking: should "0" really be the minimum tested against? It seems to me that a small minimum amount of disk space is required for sessions to work, say, 1k or something. So maybe the test should be

if (!($freespace > 1024))

or something. Just a suggestion. In this day and age, 1024 practically is zero anyway.

thanks, seeya

Show
Jeffrey Silverman added a comment - Howdy. The patch seems reasonable, (as long as disk_free_space() returns false on fail, obviously). I'll try a test and report back. Also, I was thinking: should "0" really be the minimum tested against? It seems to me that a small minimum amount of disk space is required for sessions to work, say, 1k or something. So maybe the test should be if (!($freespace > 1024)) or something. Just a suggestion. In this day and age, 1024 practically is zero anyway. thanks, seeya
Hide
Permalink
Jeffrey Silverman added a comment -

The patch works for us. Thanks!

Show
Jeffrey Silverman added a comment - The patch works for us. Thanks!
Hide
Permalink
Petr Škoda (skodak) added a comment -

you could make it 1 megabyte, anything below this is not going to end well anyway

Show
Petr Škoda (skodak) added a comment - you could make it 1 megabyte, anything below this is not going to end well anyway
Hide
Permalink
Petr Škoda (skodak) added a comment -

silly question, is the free disk space cached somehow? I am a bit worried about major performance regressions. Maybe we could just test if session file exists, that should be cached and file locks should not matter, right?

Show
Petr Škoda (skodak) added a comment - silly question, is the free disk space cached somehow? I am a bit worried about major performance regressions. Maybe we could just test if session file exists, that should be cached and file locks should not matter, right?
Hide
Permalink
Francois Marier added a comment -

I applied the patch tested by Jeffrey. However I bumped the number from 0 to 2048.

I'm not sure about bumping it up to 1MB though because you can login and do a lot of stuff with much less free space than this

As for the possible performance impact, this would need to be tested on different filesystems to compare the cost of checking the available space versus the cost of checking to see if a file exists.

Show
Francois Marier added a comment - I applied the patch tested by Jeffrey. However I bumped the number from 0 to 2048. I'm not sure about bumping it up to 1MB though because you can login and do a lot of stuff with much less free space than this As for the possible performance impact, this would need to be tested on different filesystems to compare the cost of checking the available space versus the cost of checking to see if a file exists.
Hide
Permalink
Petr Škoda (skodak) added a comment -

More reports are popping up in moodle.org forums, please revert the patch in STABLE asap.

The problem is:
1/ if it for some reason does not work it halts the site completely
2/ it should not be imo tested in setup.php on each page, instead we could implement advanced diagnostics only when session times out

In any case I do not think it is a good idea to experiment with this in stable

Show
Petr Škoda (skodak) added a comment - More reports are popping up in moodle.org forums, please revert the patch in STABLE asap. The problem is: 1/ if it for some reason does not work it halts the site completely 2/ it should not be imo tested in setup.php on each page, instead we could implement advanced diagnostics only when session times out In any case I do not think it is a good idea to experiment with this in stable
Hide
Permalink
Francois Marier added a comment -

Reverted in 1.9 stable.

Feel free to review, change or revert it in HEAD.

Show
Francois Marier added a comment - Reverted in 1.9 stable. Feel free to review, change or revert it in HEAD.

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.