[vmstore/main] Use property list for results so they work cross-session

5 files changed, 84 insertions, 45 deletions

diff --git a/inc/property.inc.php b/inc/property.inc.php
index 734c559e..e17473f6 100644
--- a/inc/property.inc.php
+++ b/inc/property.inc.php
@@ -118,12 +118,12 @@ class Property
 	 * @param string $key key of list
 	 * @param int $subkey subkey of entry in list
 	 * @param string $value new value to set entry to
-	 * @param string|null $expectedValue if not null, the value will only be updated if it currently has this value
 	 * @param int $maxAgeMinutes the new lifetime of that entry
+	 * @param ?string $expectedValue if not null, the value will only be updated if it currently has this value
 	 * @return bool whether the entry existed and has been updated
 	 */
-	public static function updateListEntry(string $key, int $subkey, string $value, string $expectedValue = null,
-														int $maxAgeMinutes = 0): bool
+	public static function updateListEntry(string $key, int $subkey, string $value,
+														int $maxAgeMinutes = 0, string $expectedValue = null): bool
 	{
 		$args = [
 			'name' => $key,
diff --git a/modules-available/main/install.inc.php b/modules-available/main/install.inc.php
index a19a069d..ac38214e 100644
--- a/modules-available/main/install.inc.php
+++ b/modules-available/main/install.inc.php
@@ -21,7 +21,7 @@ $res[] = tableCreate('permission', "
 $res[] = tableCreate('property', "
 	`name` varchar(50) NOT NULL,
 	`dateline` int(10) unsigned NOT NULL DEFAULT '0',
-	`value` text NOT NULL,
+	`value` mediumblob NOT NULL,
 	PRIMARY KEY (`name`),
 	KEY `dateline` (`dateline`)
 ");
@@ -30,7 +30,7 @@ $res[] = tableCreate('property_list', "
 	`name` varchar(50) NOT NULL,
 	`subkey` int(10) unsigned NOT NULL AUTO_INCREMENT,
 	`dateline` int(10) unsigned NOT NULL DEFAULT '0',
-	`value` text NOT NULL,
+	`value` mediumblob NOT NULL,
 	KEY (`name`),
 	KEY `dateline` (`dateline`),
 	ADD KEY (`subkey`),
@@ -93,7 +93,8 @@ $res[] = tableCreate('mail_config', "
 // ##### 2014-05-28
 // Add dateline field to property table
 if (!tableHasColumn('property', 'dateline')) {
-	Database::exec("ALTER TABLE `property` ADD `dateline` INT( 10 ) UNSIGNED NOT NULL DEFAULT '0' AFTER `name` , ADD INDEX ( `dateline` )");
+	Database::exec("ALTER TABLE `property` ADD `dateline` INT( 10 ) UNSIGNED NOT NULL DEFAULT '0' AFTER `name`,
+		ADD INDEX ( `dateline` )");
 }
 
 // #######################
@@ -119,8 +120,9 @@ if (!tableHasColumn('user', 'serverid')) {
 	Database::exec("ALTER TABLE `user` ADD `serverid` int(10) unsigned NULL DEFAULT NULL");
 }
 
-//
-//
+// #######################
+// ##### 2022-07-04
+// Add subkey to property_list, make value mediumblob instead of text
 if (!tableHasColumn('property_list', 'subkey')) {
 	$ret = Database::exec("ALTER TABLE property_list
     	ADD COLUMN `subkey` int(10) unsigned NOT NULL AUTO_INCREMENT AFTER `name`,
@@ -131,6 +133,15 @@ if (!tableHasColumn('property_list', 'subkey')) {
 	}
 	$res[] = UPDATE_DONE;
 }
+foreach (['property', 'property_list'] as $table) {
+	if (stripos(tableColumnType($table, 'value'), 'mediumblob') === false) {
+		$ret = Database::exec("ALTER TABLE `$table` MODIFY `value` mediumblob NOT NULL");
+		if ($ret === false) {
+			finalResponse(UPDATE_FAILED, "Cannot change value column of $table to mediumblob: " . Database::lastError());
+		}
+		$res[] = UPDATE_DONE;
+	}
+}
 
 // Make sure that if any users exist, one of the has UID=1, otherwise if the permission module is
 // used we'd lock out everyone
diff --git a/modules-available/vmstore/inc/vmstorebenchmark.inc.php b/modules-available/vmstore/inc/vmstorebenchmark.inc.php
index 30b0c65c..10c55431 100644
--- a/modules-available/vmstore/inc/vmstorebenchmark.inc.php
+++ b/modules-available/vmstore/inc/vmstorebenchmark.inc.php
@@ -3,6 +3,8 @@
 class VmStoreBenchmark
 {
 
+	const PROP_LIST_KEY = 'vmstore.benchmark';
+
 	/**
 	 * @param string[] $machineUuids List of UUIDs
 	 * @return void
@@ -16,8 +18,8 @@ class VmStoreBenchmark
 		if ($machines === false)
 			return;
 		$machines = array_column($machines, 'machineuuid');
-		$id = mt_rand() . time();
-		Session::set('benchmark-' . $id, ['machines' => $machines], 60);
+		$id = Property::addToList(self::PROP_LIST_KEY,
+			json_encode(['machines' => $machines]), 60);
 		Util::redirect('?do=vmstore&show=benchmark&action=select&id=' . $id);
 	}
 
@@ -40,6 +42,9 @@ class VmStoreBenchmark
 		// As of 2022, RemoteExec processes 4 clients in parallel
 		$start = ceil(count($clients) / 4 + 5 + time());
 		$nfsOpt = $nfs ? '--nfs' : '';
+		// We fork off the benchmark into the background, and collect the results with another RemoteExec job
+		// when we're done. This is because RemoteExec only does four concurrent SSH connections, so if we wanted to
+		// do this the easy, synchronous way, we never could run more than four tests at the same time.
 		$command = <<<COMMAND
 (
 exec &> /dev/null < /dev/null
diff --git a/modules-available/vmstore/page.inc.php b/modules-available/vmstore/page.inc.php
index 41e7e990..8813cde0 100644
--- a/modules-available/vmstore/page.inc.php
+++ b/modules-available/vmstore/page.inc.php
@@ -107,15 +107,26 @@ class Page_VmStore extends Page
 		}
 	}
 
+	private function getJobFromId(int $id)
+	{
+		$data = Property::getListEntry(VmStoreBenchmark::PROP_LIST_KEY, $id);
+		if ($data !== null) {
+			$data = json_decode($data, true);
+		}
+		if (!is_array($data) || !isset($data['machines'])) {
+			Message::addError('invalid-benchmark-job', $id);
+			return null;
+		}
+		return $data;
+	}
+
 	private function benchmarkActionStart()
 	{
 		Module::isAvailable('dnbd3');
-		$id = Request::post('id', Request::REQUIRED, 'string');
-		$data = Session::get('benchmark-' . $id);
-		if (!isset($data['machines'])) {
-			Message::addError('invalid-benchmark-job', $id);
+		$id = Request::post('id', Request::REQUIRED, 'int');
+		$data = $this->getJobFromId($id);
+		if ($data === null)
 			return;
-		}
 		if (isset($data['task'])) {
 			if ($data['task'] === 'inprogress') {
 				// Let's hope the proper ID gets written in a short while
@@ -127,29 +138,26 @@ class Page_VmStore extends Page
 		$data['image'] = Request::post('image', Request::REQUIRED, 'string');
 		// Save once first to minimize race window
 		$data['task'] = 'inprogress';
-		Session::set('benchmark-' . $id, $data, 60);
-		Session::saveExtraData();
+		Property::updateListEntry(VmStoreBenchmark::PROP_LIST_KEY, $id, json_encode($data), 30);
 		$start = 0;
 		$data['task'] = VmStoreBenchmark::start($id, $data['machines'], $data['image'], $nfs, $start);
 		if ($data['task'] === null) {
 			$data['task'] = 'failed';
 		} else {
 			// Test is 2x 30 seconds
-			$data['expected'] = $start + 60;
+			$data['expected'] = $start + 64;
 		}
 		error_log('Saving: ' . json_encode($data));
-		Session::set('benchmark-' . $id, $data, 60);
+		Property::updateListEntry(VmStoreBenchmark::PROP_LIST_KEY, $id, json_encode($data), 30);
 		Util::redirect('?do=vmstore&show=benchmark&action=result&id=' . $id);
 	}
 
 	private function benchmarkShowImageSelect()
 	{
-		$id = Request::get('id', Request::REQUIRED, 'string');
-		$data = Session::get('benchmark-' . $id);
-		if (!isset($data['machines'])) {
-			Message::addError('invalid-benchmark-job', $id);
+		$id = Request::get('id', Request::REQUIRED, 'int');
+		$data = $this->getJobFromId($id);
+		if ($data === null)
 			return;
-		}
 		if (isset($data['task'])) {
 			Message::addWarning('benchmark-already-started');
 			Util::redirect('?do=vmstore&show=benchmark&action=result&id=' . $id);
@@ -182,12 +190,10 @@ class Page_VmStore extends Page
 
 	private function benchmarkShowResult()
 	{
-		$id = Request::get('id', Request::REQUIRED, 'string');
-		$data = Session::get('benchmark-' . $id);
-		if (!isset($data['machines'])) {
-			Message::addError('invalid-benchmark-job', $id);
+		$id = Request::get('id', Request::REQUIRED, 'int');
+		$data = $this->getJobFromId($id);
+		if ($data === null)
 			return;
-		}
 		if (!isset($data['task'])) {
 			Message::addWarning('select-image-first');
 			Util::redirect('?do=vmstore&show=benchmark&action=select&id=' . $id);
@@ -219,7 +225,7 @@ class Page_VmStore extends Page
 		Render::addTemplate('benchmark-result', $args);
 	}
 
-	private function processRunningBenchmark(string $id, array &$data, bool $timeout)
+	private function processRunningBenchmark(int $id, array &$data, bool $timeout)
 	{
 		Module::isAvailable('rebootcontrol');
 		$changed = false;
@@ -270,7 +276,7 @@ EOF;
 			$changed = true;
 		}
 		if ($changed) {
-			Session::set('benchmark-' . $id, $data);
+			Property::updateListEntry(VmStoreBenchmark::PROP_LIST_KEY, $id, json_encode($data), 30);
 		}
 	}
 
diff --git a/modules-available/vmstore/templates/benchmark-result.html b/modules-available/vmstore/templates/benchmark-result.html
index fc7f8a55..edf4a4f5 100644
--- a/modules-available/vmstore/templates/benchmark-result.html
+++ b/modules-available/vmstore/templates/benchmark-result.html
@@ -4,19 +4,13 @@
 
 {{#remaining}}
 <div class="alert alert-info">
-	{{lang_benchmarkSecondsReminaing}}: {{remaining}}
+	{{lang_benchmarkSecondsReminaing}}: <span id="remaining-seconds">{{remaining}}</span>
 </div>
 {{/remaining}}
 
 <div id="graphs"></div>
 
 <script>
-	{{#refresh}}
-		setTimeout(function() {
-			window.location.reload();
-		}, {{refresh}} * 1000);
-	{{/refresh}}
-
 	document.addEventListener('DOMContentLoaded', function() {
 		var result = {{{result}}};
 		var clients = {{{wanted}}};
@@ -30,10 +24,14 @@
 		function makeGraph(typeKey, resourceKey, caption) {
 			var uuid;
 			var ds = [];
-			var gmin = 0, lmax = 0;
+			var gmin = 0, rmax = 0;
 			var colors = [];
 			var cnt = 0;
 			for (uuid in result) {
+				if (!result[uuid][typeKey]) {
+					delete result[uuid];
+					continue;
+				}
 				if (gmin === 0 || result[uuid][typeKey].start < gmin) {
 					gmin = result[uuid][typeKey].start;
 				}
@@ -54,7 +52,7 @@
 				for (i = 0; i < v.length; ++i) {
 					v[i].x += o;
 					if (cnt > 1) {
-						idx = (v[i].x / 250) | 0;
+						idx = Math.round(v[i].x / 250);
 						if (sums[idx]) {
 							sums[idx] += v[i].y | 0;
 						} else {
@@ -62,7 +60,7 @@
 						}
 					}
 				}
-				if (v[v.length-1].x > lmax) lmax = v[v.length-1].x; // Get max value
+				if (v[v.length-1].x > rmax) rmax = v[v.length-1].x; // Get max value
 				ds.push({data: v, label: result[uuid].name, borderColor: colors[ds.length], fill: false});
 			}
 			if (cnt > 1) {
@@ -74,15 +72,14 @@
 				$e.append($('<h3>').text(caption));
 				$e.append($c);
 				var ls = [];
-				for (i = 0; i <= lmax; i += 250) ls.push(i); // Generate steps for graph
+				for (i = 0; i <= rmax; i += 250) ls.push(i); // Generate steps for graph
 				graphs[typeKey] = new Chart($c[0].getContext('2d'), {data: {datasets: ds, labels: ls}, type: 'scatter', options: {
 						animation: false,
 						responsive: true,
-						spanGaps: true,
 						borderWidth: 2,
 						pointBorderWidth: 0,
 						showLine: true,
-						scales: { y: { ticks: { callback: formatBytes }}, x: { ticks: { callback: renderX } } },
+						scales: { y: { ticks: { callback: formatBytes }}, x: { ticks: { callback: renderX }, max: rmax } },
 						plugins: {
 							tooltip: { callbacks: { label: function(context) {
 										if (context.parsed.y !== null) {
@@ -90,7 +87,9 @@
 										}
 										return context.dataset.label;
 									}
-							}}}
+							}},
+							legend: { position: 'left'}
+						}
 					}});
 			} else {
 				graphs[typeKey].data.datasets = ds;
@@ -102,5 +101,23 @@
 		makeGraph('SEQ', 'net', 'Sequential Reads');
 		makeGraph('RND', 'net', 'Random 1M');
 
+		{{#refresh}}
+			setTimeout(function() {
+				window.location.reload();
+			}, {{refresh}} * 1000);
+			{{#remaining}}
+				var remaining = {{remaining}};
+				function updateRemainingCounter() {
+					if (remaining > 0) {
+						setTimeout(updateRemainingCounter, 1000);
+					} else {
+						window.location.reload();
+					}
+					$('#remaining-seconds').text(remaining--);
+				}
+				updateRemainingCounter();
+			{{/remaining}}
+		{{/refresh}}
+
 	});
 </script>
\ No newline at end of file