[pve-devel] RRD graphics in webfrontend broken

[Stefan Priebe]

Sorry last patch still contains a bug. The code can rename a .png.tmp 
file of another process which has already started to write to that file.

So we need file locking too.

Here's a new one which uses a lock file using file lock (flock). If we 
die the $LOCK goes out of scope and gets closed automatically.

--- /usr/share/perl5/PVE/Cluster.pm     2012-09-24 06:07:04.000000000 +0200
+++ Cluster.pm  2012-10-30 21:18:47.808948200 +0100
@@ -670,7 +670,7 @@ sub create_rrd_graph {

      my $rrd = "$rrddir/$rrdname";

-    my $filename = "$rrd.png";
+    my $filename = "${rrd}_$ds.png";

      my $setup = {
         hour =>  [ 60, 60 ],
@@ -714,11 +714,19 @@ sub create_rrd_graph {

      push @args, '--full-size-mode';

-    RRDs::graph($filename, @args);
+    # lock and flock file to be sure we're the only
+    # one generating a new graph for this $rrdname and $ds
+    open(my $LOCK, ">", $filename.".lock");
+    flock $LOCK, 2;
+
+    RRDs::graph($filename.".tmp", @args);

      my $err = RRDs::error;
      die "RRD error: $err\n" if $err;

+    rename $filename.".tmp", $filename;
+    close($LOCK);
+
      return { filename => $filename };
  }

Stefan

Am 30.10.2012 20:13, schrieb Stefan Priebe:
> Am 30.10.2012 18:14, schrieb Dietmar Maurer:
>>> I think it's here :
>>>
>>> /usr/share/perl5/PVE/Cluster.pm
>>>
>>> sub create_rrd_graph {
>>
>> I guess I have also seen that.
>>
>> The big plan is to use ExtJS graph feature to display rrd data. But
>> that is a considerable amount of work (debug ExtJS code).
>
> nice but i can't do this.
>
> But here is a fix for the problem. The problem is that we write to ONE
> .png file ALL graphs for different ds. So this splits the destination
> .png file per ds. Also it fixes the problem where ExtJS / the browser
> loads a .png file which is not fully written by using a tmp file and
> then rename it.
>
> --- /usr/share/perl5/PVE/Cluster.pm     2012-09-24 06:07:04.000000000 +0200
> +++ Cluster.pm  2012-10-30 20:08:52.355973920 +0100
> @@ -670,7 +670,7 @@ sub create_rrd_graph {
>
>       my $rrd = "$rrddir/$rrdname";
>
> -    my $filename = "$rrd.png";
> +    my $filename = "${rrd}_$ds.png";
>
>       my $setup = {
>          hour =>  [ 60, 60 ],
> @@ -714,11 +714,12 @@ sub create_rrd_graph {
>
>       push @args, '--full-size-mode';
>
> -    RRDs::graph($filename, @args);
> +    RRDs::graph($filename.".tmp", @args);
>
>       my $err = RRDs::error;
>       die "RRD error: $err\n" if $err;
>
> +    rename $filename.".tmp", $filename;
>       return { filename => $filename };
>   }
>
>
> Stefan
>
>