Skip to content

Commit

Permalink
Attempt to fix flakiness
Browse files Browse the repository at this point in the history
  • Loading branch information
kertal committed Oct 7, 2024
1 parent 7d4e3da commit d574963
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,19 @@ import type { DiscoverStateContainer } from '../../state_management/discover_sta
import { addLog } from '../../../../utils/add_log';
import { useInternalStateSelector } from '../../state_management/discover_internal_state_container';
import type { DiscoverAppState } from '../../state_management/discover_app_state_container';
import { DataDocumentsMsg } from '../../state_management/discover_data_state_container';
import type { DataDocumentsMsg } from '../../state_management/discover_data_state_container';
import { useSavedSearch } from '../../state_management/discover_state_provider';
import { useIsEsqlMode } from '../../hooks/use_is_esql_mode';

const EMPTY_ESQL_COLUMNS: DatatableColumn[] = [];
const EMPTY_FILTERS: Filter[] = [];

// enabled for debugging
if (window) {
// @ts-ignore
window.ELASTIC_DISCOVER_LOGGER = `debug`;
}

export interface UseDiscoverHistogramProps {
stateContainer: DiscoverStateContainer;
inspectorAdapters: InspectorAdapters;
Expand Down Expand Up @@ -180,17 +186,45 @@ export const useDiscoverHistogram = ({
totalHitsResult &&
typeof result !== 'number'
) {
// ignore the histogram initial loading state if discover state already has a total hits value
addLog(
'[UnifiedHistogram] ignore the histogram initial loading state if discover state already has a total hits value',
{ status, result }
);
return;
}

const fetchStatus = status.toString() as FetchStatus;
if (
fetchStatus === FetchStatus.COMPLETE &&
savedSearchData$.totalHits$.getValue().fetchStatus === FetchStatus.COMPLETE &&
result !== savedSearchData$.totalHits$.getValue().result
) {
// this is a workaround to make sure the new total hits value is displayed
// a different value without a loading state in between would lead to be ignored by useDataState
addLog(
'[UnifiedHistogram] send loading state to total hits$ to make sure the new value is displayed',
{ status, result }
);
savedSearchData$.totalHits$.next({
fetchStatus: FetchStatus.LOADING,
});
}

// Sync the totalHits$ observable with the unified histogram state
savedSearchData$.totalHits$.next({
fetchStatus: status.toString() as FetchStatus,
fetchStatus,
result,
});

if (status !== UnifiedHistogramFetchStatus.complete || typeof result !== 'number') {
if (
(status !== UnifiedHistogramFetchStatus.complete &&
status !== UnifiedHistogramFetchStatus.partial) ||
typeof result !== 'number'
) {
addLog(
'[UnifiedHistogram] ignore the histogram complete/partial state if discover state already has a total hits value',
{ status, result }
);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ describe('test fetchAll', () => {
getAppState: () => ({}),
getInternalState: () => ({
dataView: undefined,
isLoading: false,
isDataViewLoading: false,
savedDataViews: [],
adHocDataViews: [],
Expand Down Expand Up @@ -262,6 +263,7 @@ describe('test fetchAll', () => {
getInternalState: () => ({
dataView: undefined,
isDataViewLoading: false,
isLoading: false,
savedDataViews: [],
adHocDataViews: [],
expandedDoc: undefined,
Expand Down Expand Up @@ -384,6 +386,7 @@ describe('test fetchAll', () => {
getInternalState: () => ({
dataView: undefined,
isDataViewLoading: false,
isLoading: false,
savedDataViews: [],
adHocDataViews: [],
expandedDoc: undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,14 +369,14 @@ export function DiscoverMainLoading({
mainContent: JSX.Element;
}) {
const loading = useInternalStateSelector((state) => state.isLoading);
if (loading && !showNoDataPage) {
return <LoadingIndicator />;
}

return (
<>
<DiscoverTopNavInline stateContainer={stateContainer} hideNavMenuItems={showNoDataPage} />
{mainContent}
<DiscoverTopNavInline
stateContainer={stateContainer}
hideNavMenuItems={showNoDataPage || loading}
/>
{loading && !showNoDataPage ? <LoadingIndicator /> : mainContent}
</>
);
}
Expand Down

0 comments on commit d574963

Please sign in to comment.