Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discover] Enhance Discover loading time by parallel execution of requests #195670

Merged
merged 10 commits into from
Oct 11, 2024
Merged

[Discover] Enhance Discover loading time by parallel execution of requests #195670

Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a479ef0
Execute intial requests in Discover main in parallel
kertal Oct 9, 2024
bfbaa9c
Refactor code
kertal Oct 10, 2024
c7b50fb
Cleanup code
kertal Oct 10, 2024
d44cc49
Update src/plugins/discover/public/application/main/discover_main_rou…
kertal Oct 11, 2024
59d004f
Update src/plugins/discover/public/application/main/discover_main_rou…
kertal Oct 11, 2024
1f6a8fe
Update src/plugins/discover/public/application/main/discover_main_rou…
kertal Oct 11, 2024
82bd512
Merge branch 'main' into discover-initial-requests
kertal Oct 11, 2024
0068625
fix linting
kertal Oct 11, 2024
0c1c831
Merge branch 'main' into discover-initial-requests
kertal Oct 11, 2024
0d1ff63
Merge branch 'main' into discover-initial-requests
kertal Oct 11, 2024
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
112 changes: 58 additions & 54 deletions src/plugins/discover/public/application/main/discover_main_route.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,11 @@ export function DiscoverMainRoute({
});
const [error, setError] = useState<Error>();
const [loading, setLoading] = useState(true);
const [hasESData, setHasESData] = useState(false);
const [hasUserDataView, setHasUserDataView] = useState(false);
const [showNoDataPage, setShowNoDataPage] = useState<boolean>(false);
const [noDataState, setNoDataState] = useState({
hasESData: true,
hasUserDataView: true,
kertal marked this conversation as resolved.
Show resolved Hide resolved
showNoDataPage: 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.

A bit of optimization, so let setState is being called, this will lead to less re-renderings, however those are just small wins

const hasCustomBranding = useObservable(core.customBranding.hasCustomBranding$, false);

/**
Expand All @@ -109,46 +111,48 @@ export function DiscoverMainRoute({
page: 'app',
id: savedSearchId || 'new',
});
/**
* Helper function to determine when to skip the no data page
*/
const skipNoDataPage = useCallback(
async (nextDataView: DataView | undefined) => {
try {
const isEsqlQuery = isDataSourceType(
stateContainer.appState.getState().dataSource,
DataSourceType.Esql
);
const skipDataTest = savedSearchId || isEsqlQuery;

if (nextDataView || skipDataTest) {
if (!isEsqlQuery) {
await stateContainer.actions.loadDataViewList();
}
return true;
}

const checkData = useCallback(async () => {
try {
if (savedSearchId) {
return true; // bypass NoData screen
}

if (isDataSourceType(stateContainer.appState.getState().dataSource, DataSourceType.Esql)) {
const [hasUserDataViewValue, hasESDataValue, defaultDataViewExists] = await Promise.all([
data.dataViews.hasData.hasUserDataView().catch(() => false),
data.dataViews.hasData.hasESData().catch(() => false),
data.dataViews.defaultDataViewExists().catch(() => false),
await stateContainer.actions.loadDataViewList(),
]);
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 is the main improvement. Execution in parallel is beneficial for loading performance


if (!hasUserDataViewValue || !defaultDataViewExists) {
setNoDataState({
showNoDataPage: true,
hasESData: hasESDataValue,
hasUserDataView: hasUserDataViewValue,
});
return false;
}
return true;
}

const hasUserDataViewValue = await data.dataViews.hasData
.hasUserDataView()
.catch(() => false);
const hasESDataValue = await data.dataViews.hasData.hasESData().catch(() => false);
setHasUserDataView(hasUserDataViewValue);
setHasESData(hasESDataValue);

if (!hasUserDataViewValue) {
setShowNoDataPage(true);
return false;
}

let defaultDataViewExists: boolean = false;
try {
defaultDataViewExists = await data.dataViews.defaultDataViewExists();
} catch (e) {
//
}

if (!defaultDataViewExists) {
setShowNoDataPage(true);
setError(e);
return false;
}
return true;
} catch (e) {
setError(e);
return false;
}
}, [data.dataViews, savedSearchId, stateContainer.appState]);
},
[data.dataViews, savedSearchId, stateContainer]
);

const loadSavedSearch = useCallback(
async ({
Expand All @@ -157,13 +161,12 @@ export function DiscoverMainRoute({
}: { nextDataView?: DataView; initialAppState?: LoadParams['initialAppState'] } = {}) => {
const loadSavedSearchStartTime = window.performance.now();
setLoading(true);
if (!nextDataView && !(await checkData())) {
const skipNoData = await skipNoDataPage(nextDataView);
if (!skipNoData) {
setLoading(false);
return;
}
try {
await stateContainer.actions.loadDataViewList();

const currentSavedSearch = await stateContainer.actions.loadSavedSearch({
savedSearchId,
dataView: nextDataView,
Expand Down Expand Up @@ -214,8 +217,8 @@ export function DiscoverMainRoute({
}
},
[
checkData,
stateContainer.actions,
skipNoDataPage,
stateContainer,
savedSearchId,
historyLocationState?.dataViewSpec,
customizationContext.displayMode,
Expand All @@ -231,11 +234,12 @@ export function DiscoverMainRoute({

useEffect(() => {
if (!isCustomizationServiceInitialized) return;

setLoading(true);
setHasESData(false);
setHasUserDataView(false);
setShowNoDataPage(false);
setNoDataState({
hasESData: false,
hasUserDataView: false,
showNoDataPage: false,
});
setError(undefined);
if (savedSearchId) {
loadSavedSearch();
Expand All @@ -259,7 +263,7 @@ export function DiscoverMainRoute({
async (nextDataView: unknown) => {
if (nextDataView) {
setLoading(true);
setShowNoDataPage(false);
setNoDataState({ showNoDataPage: false, hasESData: true, hasUserDataView: true });
kertal marked this conversation as resolved.
Show resolved Hide resolved
setError(undefined);
await loadSavedSearch({ nextDataView: nextDataView as DataView });
}
Expand All @@ -281,15 +285,15 @@ export function DiscoverMainRoute({

// We've already called this, so we can optimize the analytics services to
// use the already-retrieved data to avoid a double-call.
hasESData: () => Promise.resolve(hasESData),
hasUserDataView: () => Promise.resolve(hasUserDataView),
hasESData: () => Promise.resolve(noDataState.hasESData),
hasUserDataView: () => Promise.resolve(noDataState.hasUserDataView),
},
},
share,
dataViewEditor,
noDataPage: services.noDataPage,
}),
[core, data.dataViews, dataViewEditor, hasESData, hasUserDataView, services.noDataPage, share]
[core, data.dataViews, dataViewEditor, noDataState, services.noDataPage, share]
);

const loadingIndicator = useMemo(
Expand All @@ -298,7 +302,7 @@ export function DiscoverMainRoute({
);

const mainContent = useMemo(() => {
if (showNoDataPage) {
if (noDataState.showNoDataPage) {
const importPromise = import('@kbn/shared-ux-page-analytics-no-data');
const AnalyticsNoDataPageKibanaProvider = withSuspense(
React.lazy(() =>
Expand Down Expand Up @@ -336,7 +340,7 @@ export function DiscoverMainRoute({
noDataDependencies,
onDataViewCreated,
onESQLNavigationComplete,
showNoDataPage,
noDataState,
kertal marked this conversation as resolved.
Show resolved Hide resolved
stateContainer,
]);

Expand All @@ -357,7 +361,7 @@ export function DiscoverMainRoute({
<>
<DiscoverTopNavInline
stateContainer={stateContainer}
hideNavMenuItems={loading || showNoDataPage}
hideNavMenuItems={loading || noDataState.showNoDataPage}
/>
{mainContent}
</>
Expand Down