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.

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

Handle slow save requests better #4593

Merged
merged 11 commits into from May 11, 2020
2 changes: 1 addition & 1 deletion CHANGELOG.unreleased.md
Expand Up @@ -16,7 +16,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released

### Changed

-
- Improved logging in case of very slow annotation saving. Additionally, the user is also warned when there are unsaved changes older than two minutes. [#4593](https://github.com/scalableminds/webknossos/pull/4593)

### Fixed

Expand Down
17 changes: 16 additions & 1 deletion frontend/javascripts/oxalis/model/sagas/save_saga.js
Expand Up @@ -49,6 +49,7 @@ import compactSaveQueue from "oxalis/model/helpers/compaction/compact_save_queue
import compactUpdateActions from "oxalis/model/helpers/compaction/compact_update_actions";
import messages from "messages";
import window, { alert, document, location } from "libs/window";
import ErrorHandling from "libs/error_handling";

import { enforceSkeletonTracing } from "../accessors/skeletontracing_accessor";

Expand Down Expand Up @@ -176,16 +177,26 @@ export function* sendRequestToServer(tracingType: "skeleton" | "volume"): Saga<v
let retryCount = 0;
while (true) {
try {
const startTime = Date.now();
yield* call(
sendRequestWithToken,
`${tracingStoreUrl}/tracings/${type}/${tracingId}/update?token=`,
{
method: "POST",
headers: { "X-Date": `${Date.now()}` },
data: compactedSaveQueue,
compress: true,
},
);
const endTime = Date.now();
if (endTime - startTime > PUSH_THROTTLE_TIME) {
yield* call(
[ErrorHandling, ErrorHandling.notify],
new Error(
`Warning: Save request took more than ${Math.ceil(PUSH_THROTTLE_TIME / 1000)} seconds.`,
),
);
}

yield* put(setVersionNumberAction(version + compactedSaveQueue.length, tracingType));
yield* put(setLastSaveTimestampAction(tracingType));
yield* put(shiftSaveQueueAction(saveQueue.length, tracingType));
Expand All @@ -196,6 +207,10 @@ export function* sendRequestToServer(tracingType: "skeleton" | "volume"): Saga<v
if (error.status === 409) {
// HTTP Code 409 'conflict' for dirty state
window.onbeforeunload = null;
yield* call(
[ErrorHandling, ErrorHandling.notify],
new Error("Saving failed due to '409' status code"),
);
philippotto marked this conversation as resolved.
Show resolved Hide resolved
yield* call(alert, messages["save.failed_simultaneous_tracing"]);
location.reload();
return;
Expand Down
61 changes: 59 additions & 2 deletions frontend/javascripts/oxalis/view/action-bar/save_button.js
@@ -1,12 +1,15 @@
// @flow
import { connect } from "react-redux";
import React from "react";
import _ from "lodash";

import type { OxalisState, ProgressInfo, IsBusyInfo } from "oxalis/store";
import { isBusy } from "oxalis/model/accessors/save_accessor";
import ButtonComponent from "oxalis/view/components/button_component";
import Model from "oxalis/model";
import window from "libs/window";
import { Tooltip, Icon } from "antd";
import ErrorHandling from "libs/error_handling";

type OwnProps = {|
onClick: (SyntheticInputEvent<HTMLButtonElement>) => Promise<*>,
Expand All @@ -15,19 +18,34 @@ type OwnProps = {|
type StateProps = {|
progressInfo: ProgressInfo,
isBusyInfo: IsBusyInfo,
oldestUnsavedTimestamp: ?number,
|};
type Props = {| ...OwnProps, ...StateProps |};

type State = {
isStateSaved: boolean,
currentTimestamp: number,
};

const SAVE_POLLING_INTERVAL = 1000;
const SAVE_POLLING_INTERVAL = 1000; // 1s
const UNSAVED_WARNING_THRESHOLD = 2 * 60 * 1000; // 2 min
const REPORT_THROTTLE_THRESHOLD = 10 * 60 * 1000; // 10 min

const reportUnsavedDurationThresholdExceeded = _.throttle(() => {
ErrorHandling.notify(
new Error(
`Warning: Saving lag detected. Some changes are unsaved and older than ${Math.ceil(
UNSAVED_WARNING_THRESHOLD / 1000 / 60,
)} minutes.`,
),
);
}, REPORT_THROTTLE_THRESHOLD);

class SaveButton extends React.PureComponent<Props, State> {
savedPollingInterval: number = 0;
state = {
isStateSaved: false,
currentTimestamp: Date.now(),
};

componentDidMount() {
Expand All @@ -43,6 +61,7 @@ class SaveButton extends React.PureComponent<Props, State> {
const isStateSaved = Model.stateSaved();
this.setState({
isStateSaved,
currentTimestamp: Date.now(),
});
};

Expand All @@ -62,14 +81,23 @@ class SaveButton extends React.PureComponent<Props, State> {
}

render() {
const { progressInfo } = this.props;
const { progressInfo, oldestUnsavedTimestamp } = this.props;
const unsavedDuration =
oldestUnsavedTimestamp != null ? this.state.currentTimestamp - oldestUnsavedTimestamp : 0;
const showUnsavedWarning = unsavedDuration > UNSAVED_WARNING_THRESHOLD;
if (showUnsavedWarning) {
reportUnsavedDurationThresholdExceeded();
}
console.log("The oldest unsaved action was", unsavedDuration, "ago");

return (
<ButtonComponent
key="save-button"
type="primary"
onClick={this.props.onClick}
icon={this.getSaveButtonIcon()}
className={this.props.className}
style={{ background: showUnsavedWarning ? "#e33f36" : null }}
>
{this.shouldShowProgress() ? (
<span style={{ marginLeft: 8 }}>
Expand All @@ -79,16 +107,45 @@ class SaveButton extends React.PureComponent<Props, State> {
) : (
<span className="hide-on-small-screen">Save</span>
)}
{showUnsavedWarning ? (
<Tooltip
visible
title={`There are unsaved changes which are older than ${Math.ceil(
UNSAVED_WARNING_THRESHOLD / 1000 / 60,
)} minutes. Please ensure that your Internet connection works and wait until this warning disappears.`}
placement="bottom"
>
<Icon type="exclamation-circle" />
</Tooltip>
) : null}
</ButtonComponent>
);
}
}

function getOldestUnsavedTimestamp(saveQueue): ?number {
let oldestUnsavedTimestamp;
if (saveQueue.skeleton.length > 0) {
oldestUnsavedTimestamp = saveQueue.skeleton[0].timestamp;
}
if (saveQueue.volume.length > 0) {
const oldestVolumeTimestamp = saveQueue.volume[0].timestamp;
oldestUnsavedTimestamp = Math.min(
oldestUnsavedTimestamp != null ? oldestUnsavedTimestamp : Infinity,
oldestVolumeTimestamp,
);
}
return oldestUnsavedTimestamp;
}

function mapStateToProps(state: OxalisState): StateProps {
const { progressInfo, isBusyInfo } = state.save;
const oldestUnsavedTimestamp = getOldestUnsavedTimestamp(state.save.queue);

return {
progressInfo,
isBusyInfo,
oldestUnsavedTimestamp,
};
}

Expand Down
Expand Up @@ -186,7 +186,6 @@ async function sendUpdateActions(explorational, queue) {
`${explorational.tracingStore.url}/tracings/skeleton/${skeletonTracingId}/update?token=`,
{
method: "POST",
headers: { "X-Date": "123456789" },
data: queue,
compress: false,
},
Expand Down
4 changes: 1 addition & 3 deletions frontend/javascripts/test/sagas/save_saga.spec.js
Expand Up @@ -128,7 +128,6 @@ test("SaveSaga should send request to server", t => {
saga.next(TRACINGSTORE_URL),
call(sendRequestWithToken, `${TRACINGSTORE_URL}/tracings/skeleton/1234567890/update?token=`, {
method: "POST",
headers: { "X-Date": `${TIMESTAMP}` },
data: saveQueueWithVersions,
compress: true,
}),
Expand All @@ -146,7 +145,6 @@ test("SaveSaga should retry update actions", t => {
`${TRACINGSTORE_URL}/tracings/skeleton/1234567890/update?token=`,
{
method: "POST",
headers: { "X-Date": `${TIMESTAMP}` },
data: saveQueueWithVersions,
compress: true,
},
Expand Down Expand Up @@ -181,13 +179,13 @@ test("SaveSaga should escalate on permanent client error update actions", t => {
saga.next(TRACINGSTORE_URL),
call(sendRequestWithToken, `${TRACINGSTORE_URL}/tracings/skeleton/1234567890/update?token=`, {
method: "POST",
headers: { "X-Date": `${TIMESTAMP}` },
data: saveQueueWithVersions,
compress: true,
}),
);

saga.throw({ status: 409 });
saga.next(); // error reporting
const alertEffect = saga.next().value;
t.is(alertEffect.payload.fn, alert);
t.true(saga.next().done);
Expand Down
1 change: 1 addition & 0 deletions tools/proxy/proxy.js
Expand Up @@ -71,6 +71,7 @@ for (const [key, proc] of Object.entries(processes).filter(x => x[1] !== null))
process.on("SIGTERM", killAll);

proxy.on("error", (err, req, res) => {
console.error("### Sending Bad gateway due to the following error: ", err);
res.writeHead(503);
res.end("Bad gateway");
});
Expand Down