Skip to content

Commit

Permalink
fix: improve trial log request cancelling (#8787)
Browse files Browse the repository at this point in the history
this fixes an issue where resetting the filter settings on trials with
logs that take a long time to respond (over 500 ms) can cause the catch
up log request to get cancelled. To fix, we skip aborting the request
when handling filter changes if the filter is being updated to an empty
object. This is because the request was already reset when the filter
was reset. We also ensure that old requests don't stick around by
merging the outer abort controller with the one that the stream config
gives us.

(cherry picked from commit bb88b01)
  • Loading branch information
ashtonG authored and determined-ci committed Feb 2, 2024
1 parent 55b5bd4 commit e23e162
Showing 1 changed file with 21 additions and 1 deletion.
22 changes: 21 additions & 1 deletion webui/react/src/pages/TrialDetails/TrialDetailsLogs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,20 @@ export interface Props {

type OrderBy = 'ORDER_BY_UNSPECIFIED' | 'ORDER_BY_ASC' | 'ORDER_BY_DESC';

const mergeAbortControllers = (...controllers: AbortController[]) => {
const mergedController = new AbortController();

controllers.forEach((c) => {
const abort = () => {
mergedController.abort();
c.signal.removeEventListener('abort', abort);
};
c.signal.addEventListener('abort', abort);
});

return mergedController;
};

const TrialDetailsLogs: React.FC<Props> = ({ experiment, trial }: Props) => {
const { ui } = useUI();
const [filterOptions, setFilterOptions] = useState<Filters>({});
Expand All @@ -47,6 +61,10 @@ const TrialDetailsLogs: React.FC<Props> = ({ experiment, trial }: Props) => {

const handleFilterChange = useCallback(
(filters: Filters) => {
// request should have already been canceled when resetSettings updated
// the settings hash
if (Object.keys(filters).length === 0) return;

canceler.current.abort();
const newCanceler = new AbortController();
canceler.current = newCanceler;
Expand Down Expand Up @@ -104,6 +122,8 @@ const TrialDetailsLogs: React.FC<Props> = ({ experiment, trial }: Props) => {

const handleFetch = useCallback(
(config: FetchConfig, type: FetchType) => {
const { signal } = mergeAbortControllers(config.canceler, canceler.current);

const options = {
follow: false,
limit: config.limit,
Expand Down Expand Up @@ -142,7 +162,7 @@ const TrialDetailsLogs: React.FC<Props> = ({ experiment, trial }: Props) => {
options.timestampAfter ? new Date(options.timestampAfter) : undefined,
options.orderBy as OrderBy,
settings.searchText,
{ signal: canceler.current.signal },
{ signal },
);
},
[settings, trial?.id],
Expand Down

0 comments on commit e23e162

Please sign in to comment.