Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion shell/components/SortableTable/sorting.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export default {

if ( markedColumn ) {
this._defaultSortBy = markedColumn.name;
descending = markedColumn.defaultSortDescending;
descending = markedColumn.defaultSortDescending || false;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fixes a console.warning related to initial sort order being undefined in sortable table

} else if ( nameColumn ) {
// Use the name column if there is one
this._defaultSortBy = nameColumn.name;
Expand Down
57 changes: 41 additions & 16 deletions shell/components/nav/TopLevelMenu.helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ interface UpdateArgs {
searchTerm: string,
pinnedIds: string[],
unPinnedMax?: number,
forceWatch?: boolean
forceWatch?: boolean,
mgmtClusterRevision?: string,
provClusterRevision?: string,
}

type MgmtCluster = {
Expand Down Expand Up @@ -179,7 +181,7 @@ export class TopLevelMenuHelperPagination extends BaseTopLevelMenuHelper impleme
// No need to monitor for changes, the UNPINNED request will handle it
this.clustersPinnedWrapper = new PaginationWrapper({
$store,
id: 'tlm-pinned-clusters',
id: 'top-level-menu-pinned-clusters',
enabledFor: {
store: STORE.MANAGEMENT,
resource: {
Expand All @@ -192,13 +194,19 @@ export class TopLevelMenuHelperPagination extends BaseTopLevelMenuHelper impleme
// Fetch all UNPINNED clusters capped at 10 (see `clustersOthers` description for details)
this.clustersOthersWrapper = new PaginationWrapper({
$store,
id: 'tlm-unpinned-clusters',
onChange: async({ forceWatch }) => {
if (this.args) {
id: 'top-level-menu-unpinned-clusters',
onChange: async({ forceWatch, revision }) => {
if (!this.args) {
return;
}
try {
await this.update({
...this.args,
forceWatch
forceWatch,
mgmtClusterRevision: revision,
});
} catch {
// Failures should be logged lower down, not much we can do here except catch to prevent whole ui page warnings in dev mode
}
},
enabledFor: {
Expand All @@ -213,13 +221,19 @@ export class TopLevelMenuHelperPagination extends BaseTopLevelMenuHelper impleme
// Fetch all prov clusters for the mgmt clusters we have
this.provClusterWrapper = new PaginationWrapper({
$store,
id: 'tlm-prov-clusters',
onChange: async({ forceWatch }) => {
if (this.args) {
id: 'top-level-menu-prov-clusters',
onChange: async({ forceWatch, revision }) => {
if (!this.args) {
return;
}
try {
await this.update({
...this.args,
forceWatch
forceWatch,
provClusterRevision: revision,
});
} catch {
// Failures should be logged lower down, not much we can do here except catch to prevent whole ui page warnings in dev mode
}
},
enabledFor: {
Expand Down Expand Up @@ -251,7 +265,8 @@ export class TopLevelMenuHelperPagination extends BaseTopLevelMenuHelper impleme
pinned: MgmtCluster[],
notPinned: MgmtCluster[]
} = await allHash(promises) as any;
const provClusters = await this.updateProvCluster(res.notPinned, res.pinned, args.forceWatch || false);

const provClusters = await this.updateProvCluster(res.notPinned, res.pinned, args);
const provClustersByMgmtId = provClusters.reduce((res: { [mgmtId: string]: ProvCluster}, provCluster: ProvCluster) => {
if (provCluster.mgmtClusterId) {
res[provCluster.mgmtClusterId] = provCluster;
Expand Down Expand Up @@ -357,7 +372,9 @@ export class TopLevelMenuHelperPagination extends BaseTopLevelMenuHelper impleme
sort: DEFAULT_SORT,
projectsOrNamespaces: []
},
}).then((r) => r.data);
revision: args.mgmtClusterRevision
})
.then((r) => r.data);
}

/**
Expand All @@ -378,15 +395,17 @@ export class TopLevelMenuHelperPagination extends BaseTopLevelMenuHelper impleme
sort: DEFAULT_SORT,
projectsOrNamespaces: []
},
}).then((r) => r.data);
revision: args.mgmtClusterRevision
})
.then((r) => r.data);
}

/**
* Find all provisioning clusters associated with the displayed mgmt clusters
*/
private async updateProvCluster(notPinned: MgmtCluster[], pinned: MgmtCluster[], forceWatch: boolean): Promise<ProvCluster[]> {
private async updateProvCluster(notPinned: MgmtCluster[], pinned: MgmtCluster[], args: UpdateArgs): Promise<ProvCluster[]> {
return this.provClusterWrapper.request({
forceWatch,
forceWatch: args.forceWatch,
pagination: {
filters: [
PaginationParamFilter.createMultipleFields(
Expand All @@ -400,7 +419,9 @@ export class TopLevelMenuHelperPagination extends BaseTopLevelMenuHelper impleme
sort: [],
projectsOrNamespaces: []
},
}).then((r) => r.data);
revision: args.provClusterRevision
})
.then((r) => r.data);
}
}

Expand Down Expand Up @@ -585,6 +606,8 @@ export class TopLevelMenuHelperLegacy extends BaseTopLevelMenuHelper implements
*/
class TopLevelMenuHelperService {
private _helper?: TopLevelMenuHelper;
public initialized = false;

public init($store: VuexStore) {
if (this._helper) {
return;
Expand All @@ -599,6 +622,8 @@ class TopLevelMenuHelperService {
});

this._helper = canPagination ? new TopLevelMenuHelperPagination({ $store }) : new TopLevelMenuHelperLegacy({ $store });

this.initialized = true;
}

public async reset() {
Expand Down
70 changes: 45 additions & 25 deletions shell/components/nav/TopLevelMenu.vue
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ export default {
},

data() {
const sideNavServiceInitialized = sideNavService.initialized;
const maxClustersToShow = MENU_MAX_CLUSTERS;

sideNavService.init(this.$store);

const { displayVersion, fullVersion } = getVersionInfo(this.$store);
Expand All @@ -43,26 +46,27 @@ export default {
const provClusters = !canPagination && hasProvCluster ? this.$store.getters[`management/all`](CAPI.RANCHER_CLUSTER) : [];
const mgmtClusters = !canPagination ? this.$store.getters[`management/all`](MANAGEMENT.CLUSTER) : [];

if (!canPagination) {
// Reduce the impact of the initial load, but only if we're not making a request
if (!canPagination || !sideNavServiceInitialized) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix multiple request to fetch clusters when using side nav to move around

  • When changing page the header + top level menu component is recreated. The impact of this is reduced a bit given the TopLevelMenuHelperService singleton, but multiple requests are still made
  • This is due to the immediate on a number of watches in TopLevelMenu triggering new requests
  • This is now fixed by making an initial request up front (only when needed) and not triggering the watches when we don't need to

// Reduce the impact of the initial load, or properly initialised
// Doing this here means we don't need an 'immediate' on the watches below
const args = {
pinnedIds: this.pinnedIds,
searchTerm: this.search,
unPinnedMax: this.maxClustersToShow
pinnedIds: this.$store.getters['prefs/get'](PINNED_CLUSTERS),
searchTerm: '',
unPinnedMax: maxClustersToShow
};

helper.update(args);
}

return {
shown: false,
shown: false,
displayVersion,
fullVersion,
clusterFilter: '',
clusterFilter: '',
hasProvCluster,
maxClustersToShow: MENU_MAX_CLUSTERS,
emptyCluster: BLANK_CLUSTER,
routeCombo: false,
maxClustersToShow,
emptyCluster: BLANK_CLUSTER,
routeCombo: false,

canPagination,
helper,
Expand Down Expand Up @@ -284,7 +288,6 @@ export default {
// 2. When SSP is disabled (legacy) reduce fn churn (this was a known performance customer issue)

pinnedIds: {
immediate: true,
handler(neu, old) {
if (sameContents(neu, old)) {
return;
Expand All @@ -302,20 +305,28 @@ export default {

provClusters: {
handler(neu, old) {
if (this.canPagination) {
// Shouldn't be doing this at all if pagination is on (updates handled by TopLevelMenu pagination wrapper)
return;
}

// Potentially incredibly high throughput. Changes should be at least limited (slow if state change, quick if added/removed). Shouldn't get here if SSP
this.updateClusters(this.pinnedIds, neu?.length === old?.length ? 'slow' : 'quick');
},
deep: true,
immediate: true,
deep: true,
},

mgmtClusters: {
handler(neu, old) {
if (this.canPagination) {
// Shouldn't be doing this at all if pagination is on (updates handled by TopLevelMenu pagination wrapper)
return;
}

// Potentially incredibly high throughput. Changes should be at least limited (slow if state change, quick if added/removed). Shouldn't get here if SSP
this.updateClusters(this.pinnedIds, neu?.length === old?.length ? 'slow' : 'quick');
},
deep: true,
immediate: true,
deep: true,
},

hideLocalCluster() {
Expand Down Expand Up @@ -454,16 +465,25 @@ export default {
unPinnedMax: this.maxClustersToShow
};

switch (speed) {
case 'slow':
this.debouncedHelperUpdateSlow(args);
break;
case 'medium':
this.debouncedHelperUpdateMedium(args);
break;
case 'quick':
this.debouncedHelperUpdateQuick(args);
break;
try {
switch (speed) {
case 'slow':
this.debouncedHelperUpdateSlow(args);
break;
case 'medium':
this.debouncedHelperUpdateMedium(args);
break;
case 'quick':
this.debouncedHelperUpdateQuick(args);
break;
}
} catch (err) {
if (this.canPagination) {
// Double bubble up errors here, errors are tracked further down
// Note that this won't pick up async errors, further tweaks are required for that
} else {
throw err;
}
}
}
}
Expand Down
Loading