Skip to content
Permalink
Browse files

Improve web UI performance by delaying the storage node online check

  • Loading branch information
Sebastian-Roth committed Dec 1, 2019
1 parent 2275527 commit 1ff29b808939c2d104eb623a9a12c4684027ebb4
Showing with 6 additions and 8 deletions.
  1. +6 −1 packages/web/lib/fog/storagegroup.class.php
  2. +0 −7 packages/web/lib/router/route.class.php
@@ -231,7 +231,12 @@ public function getMasterStorageNode()
);
foreach ($StorageNodes->data as $StorageNode) {
if (!$StorageNode->online) {
continue;
$StorageNodeCheckOnline = new StorageNode($StorageNode->id);
if (!$StorageNodeCheckOnline->get('online')) {
unset($StorageNodeCheckOnline);
continue;
}
unset($StorageNodeCheckOnline);
}
if ($masternode == null) {
$masternode = $StorageNode;
@@ -819,13 +819,6 @@ public static function listem(
case 'storagenode':
$columns[] = ['db' => 'ngID', 'dt' => 'storagegroupID'];
$columns[] = ['db' => 'ngName', 'dt' => 'storagegroupName'];
$columns[] = [
'db' => 'ngmID',
'dt' => 'online',
'formatter' => function ($d, $row) {
return self::getClass('StorageNode', $d)->get('online');
}
];
$columns[] = [
'db' => 'ngmID',
'dt' => 'clientload',

3 comments on commit 1ff29b8

@mastacontrola

This comment has been minimized.

Copy link
Member

mastacontrola replied Dec 1, 2019

This logic is making little sense to me. You’re checking if the node is online then rechecking a tiny amount of time after. That part I understand, but you are removing the setter for online from the api, essentially this is always going to do the same thing with a slight bit more delayed. I say this as based, at least how I’m seeing this on my phone, the StorageNode->online is always going to be false.

@Sebastian-Roth

This comment has been minimized.

Copy link
Member Author

Sebastian-Roth replied Dec 2, 2019

@mastacontrola I see what you mean. The current "fix" is definitely not perfect. What I wanted to achieve is that a simple listem on storagenodes doesn't pull the online status because this is causing really high wait times in the web UI that aren't needed.

As getMasterStorageNode() is used in a few places in the code my idea was to make it a two folded check. First check $StorageNode->online because it is very efficient to only check the variable. If it's already populated in this request it will return true. But if it returns false or empty then we'd do the full online check afterwards. While it seems weird it worked quiet well in my tests.

Though I know there are still issues with this change. You can't schedule deploy tasks right now. I will test and fix that soon as well.

@mastacontrola

This comment has been minimized.

Copy link
Member

mastacontrola replied Dec 3, 2019

I've re-added the listem check for online. But I've changed two other things: I made the timeout for the isAvailable check from 1 second to 0.1 seconds. I also made the stream non-blocking in nature. This should help out with the issue hopefully while still allowing deploy tasks to operate properly.

Please sign in to comment.
You can’t perform that action at this time.