-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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.
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
feat(cache-cell-results): added cell result caching to reduce cell configuration querying on loads #18581
feat(cache-cell-results): added cell result caching to reduce cell configuration querying on loads #18581
Changes from all commits
eaa62a4
2d4c896
0c1116e
0d40fbd
c93801e
b6c0887
120dd94
3965a0c
4370a04
9a4c885
c1e7bce
69bfb40
5d6d1cd
fc2db5b
6e5a19e
014ec9e
a7e46d8
655d5de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,13 +16,18 @@ import RateLimitAlert from 'src/cloud/components/RateLimitAlert' | |
// Utils | ||
import {pageTitleSuffixer} from 'src/shared/utils/pageTitles' | ||
|
||
// Selectors | ||
// Selectors & Actions | ||
import {resetCachedQueryResults} from 'src/queryCache/actions' | ||
import {getByID} from 'src/resources/selectors' | ||
|
||
// Types | ||
import {AppState, AutoRefresh, ResourceType, Dashboard} from 'src/types' | ||
import {ManualRefreshProps} from 'src/shared/components/ManualRefresh' | ||
|
||
interface DispatchProps { | ||
resetCachedQueryResults: typeof resetCachedQueryResults | ||
} | ||
|
||
interface StateProps { | ||
dashboard: Dashboard | ||
} | ||
|
@@ -31,10 +36,14 @@ interface OwnProps { | |
autoRefresh: AutoRefresh | ||
} | ||
|
||
type Props = OwnProps & StateProps & ManualRefreshProps | ||
type Props = OwnProps & StateProps & ManualRefreshProps & DispatchProps | ||
|
||
@ErrorHandling | ||
class DashboardPage extends Component<Props> { | ||
public componentWillUnmount() { | ||
this.props.resetCachedQueryResults() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 thinking about cache invalidation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't take any credit for it, it was all @ebb-tide 's idea |
||
} | ||
|
||
public render() { | ||
const {autoRefresh, manualRefresh, onManualRefresh, children} = this.props | ||
|
||
|
@@ -76,7 +85,11 @@ const mstp = (state: AppState): StateProps => { | |
} | ||
} | ||
|
||
export default connect<StateProps, {}>( | ||
const mdtp = { | ||
resetCachedQueryResults: resetCachedQueryResults, | ||
} | ||
|
||
export default connect<StateProps, DispatchProps>( | ||
mstp, | ||
null | ||
mdtp | ||
)(ManualRefresh<OwnProps>(DashboardPage)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
export type Action = | ||
| ReturnType<typeof resetCachedQueryResults> | ||
| ReturnType<typeof setQueryResultsByQueryID> | ||
|
||
// Hashing function found here: | ||
// https://jsperf.com/hashcodelordvlad | ||
// Through this thread: | ||
// https://stackoverflow.com/questions/7616461/generate-a-hash-from-string-in-javascript | ||
export const hashCode = (queryText: string): string => { | ||
let hash = 0, | ||
char | ||
if (!queryText) { | ||
return `${hash}` | ||
} | ||
for (let i = 0; i < queryText.length; i++) { | ||
char = queryText.charCodeAt(i) | ||
hash = (hash << 5) - hash + char | ||
hash |= 0 // Convert to 32bit integer | ||
} | ||
return `${hash}` | ||
} | ||
|
||
export const setQueryResultsByQueryID = (queryID: string, files: string[]) => | ||
({ | ||
type: 'SET_QUERY_RESULTS_BY_QUERY', | ||
queryID, | ||
files, | ||
} as const) | ||
|
||
export const resetCachedQueryResults = () => | ||
({ | ||
type: 'RESET_CACHED_QUERY_RESULTS', | ||
} as const) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// Libraries | ||
import {produce} from 'immer' | ||
|
||
// Actions | ||
import {Action} from 'src/queryCache/actions' | ||
|
||
export interface QueryCacheState { | ||
queryResultsByQueryID: {[queryID: string]: string[]} | ||
} | ||
|
||
export const initialState: QueryCacheState = { | ||
queryResultsByQueryID: {}, | ||
} | ||
|
||
export const queryCacheReducer = ( | ||
state: QueryCacheState = initialState, | ||
action: Action | ||
): QueryCacheState => { | ||
switch (action.type) { | ||
case 'SET_QUERY_RESULTS_BY_QUERY': { | ||
return produce(state, draftState => { | ||
const {queryID, files} = action | ||
draftState.queryResultsByQueryID[queryID] = files | ||
}) | ||
} | ||
|
||
case 'RESET_CACHED_QUERY_RESULTS': { | ||
return {queryResultsByQueryID: {}} | ||
} | ||
} | ||
|
||
return state | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,7 +67,7 @@ interface SetQueryResults { | |
} | ||
} | ||
|
||
const setQueryResults = ( | ||
export const setQueryResults = ( | ||
status: RemoteDataState, | ||
files?: string[], | ||
fetchDuration?: number, | ||
|
@@ -125,9 +125,6 @@ export const executeQueries = (abortController?: AbortController) => async ( | |
const queries = activeTimeMachine.view.properties.queries.filter( | ||
({text}) => !!text.trim() | ||
) | ||
const { | ||
alertBuilder: {id: checkID}, | ||
} = state | ||
|
||
if (!queries.length) { | ||
dispatch(setQueryResults(RemoteDataState.Done, [], null)) | ||
|
@@ -177,6 +174,10 @@ export const executeQueries = (abortController?: AbortController) => async ( | |
) | ||
|
||
let statuses = [[]] as StatusRow[][] | ||
const { | ||
alertBuilder: {id: checkID}, | ||
} = state | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved this down so it would be place a little closer to the source of where |
||
if (checkID) { | ||
const extern = buildVarsOption(variableAssignments) | ||
pendingCheckStatuses = runStatusesQuery(getOrg(state).id, checkID, extern) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍