Dereckson has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/350200 )
Change subject: Improve CX draft saving logic
......................................................................
Improve CX draft saving logic
* Increase retry delay after each failure
* Limit automatic retry attempts to 5, after which user action is
needed to attempt saving again.
* If a save has failed, prevent other save requests other than the
the retry attempt
* Throttle auto-saving: before it would save when a three second
pause is detected. This could lead the worst case of a save
happening every three seconds. Changed this to use throttle
instead, which saves every 15 seconds (taking a very conservative
value here on purpose) regardless whether there is a pause in
the writing or not.
* Removed the unused mw.cx.translation.save hook while at it.
* More verbose debug logging to make debugging and testing easier.
These changes should reduce the amount of draft save requests
considerable both when 1) saving is failing quickly (e.g. read only)
2) save requests time out (e.g. database overloaded)
Bug: T163344
Change-Id: I37d8de3969700aa2013b74f54b976349632094ae
(cherry picked from commit 0792de585831b5982d640d010f1f59dfe530aeaf)
---
M hooks.md
M modules/translation/ext.cx.translation.storage.js
2 files changed, 49 insertions(+), 37 deletions(-)
Approvals:
Nikerabbit: Looks good to me, but someone else must approve
Dereckson: Verified; Looks good to me, approved
diff --git a/hooks.md b/hooks.md
index 6b336d3..ff858bb 100644
--- a/hooks.md
+++ b/hooks.md
@@ -145,11 +145,6 @@
Fired when the title for the translation is changed. Check for existing title
listens for this.
-## mw.cx.translation.save
-
-Fired to request saving a translation as a draft. It will check if there is
something to save.
-If so, cxsave API call will be initiated.
-
## mw.cx.translation.save-started
Fired when a cxsave API call is beginning. Can be used to indicate the
progress of save.
diff --git a/modules/translation/ext.cx.translation.storage.js
b/modules/translation/ext.cx.translation.storage.js
index 8ee04f7..808f3d8 100644
--- a/modules/translation/ext.cx.translation.storage.js
+++ b/modules/translation/ext.cx.translation.storage.js
@@ -7,7 +7,8 @@
( function ( $, mw ) {
'use strict';
- var timer, saveRequest,
+ var saveRequest,
+ timer = null,
failCounter = 0;
/**
@@ -42,12 +43,12 @@
return $content[ 0 ].outerHTML;
};
- ContentTranslationStorage.prototype.save = function () {
+ ContentTranslationStorage.prototype.save = function ( origin ) {
var sections;
sections = this.getSectionsToSave();
if ( sections && sections.length ) {
- this.saveSections( sections );
+ this.saveSections( sections, origin );
}
};
@@ -77,15 +78,11 @@
self.markForSave( $targetSection );
} );
- mw.hook( 'mw.cx.translation.change' ).add( $.debounce( 3000,
function () {
- // Reset fail counter so that autosave if stopped can
be restarted.
- failCounter = 0;
+ mw.hook( 'mw.cx.translation.change' ).add( $.throttle( 15000,
function () {
// mw.cx.translation.change get fired for every changes
in translation.
// We debounce the handler to reduce the excessive save
API calls.
- self.save();
+ self.save( 'change' );
} ) );
-
- mw.hook( 'mw.cx.translation.save' ).add( $.proxy( this.save,
this ) );
mw.hook( 'mw.cx.draft.restoring' ).add( function () {
// Do not save while restoring is being attempted
@@ -99,7 +96,7 @@
$( document ).on( 'keydown', function ( e ) {
// See
https://medium.com/medium-eng/the-curious-case-of-disappearing-polish-s-fa398313d4df
if ( ( e.metaKey || e.ctrlKey && !e.altKey ) && e.which
=== 83 ) {
- self.save();
+ self.save( 'shortcut' );
return false;
}
} );
@@ -112,7 +109,7 @@
sections = this.getSectionsToSave();
if ( sections && sections.length ) {
// Trigger save anyway.
- this.save();
+ this.save( 'navigation' );
// Inform about sections not saved.
return mw.msg( 'cx-warning-unsaved-translation' );
} else {
@@ -120,18 +117,31 @@
}
};
- ContentTranslationStorage.prototype.saveSections = function ( sections
) {
+ ContentTranslationStorage.prototype.saveSections = function ( sections,
origin ) {
var self = this,
api = new mw.Api();
+
+ mw.log( '[CX] Save request initiated by: ' + origin );
+
+ // Last save failed, and a retry has been scheduled. Don't
allow starting new
+ // save requests to avoid overloading the servers, unless this
is the retry.
+ if ( timer !== null && origin !== 'retry' ) {
+ mw.log( '[CX] Save request skipped because a retry has
been scheduled' );
+ return;
+ }
+
+ if ( saveRequest ) {
+ mw.log( '[CX] Aborted active save request' );
+ // This causes failCounter to increase because the
in-flight request fails.
+ // The new request we do below will either reset the
fail counter on success.
+ // If it does not succeed, the retry timer that was set
by the failed request
+ // prevents further saves before the retry has
completed succesfully or given up.
+ saveRequest.abort();
+ }
// Starting the real save API call. Fire event so that we can
show a progress
// indicator in UI.
mw.hook( 'mw.cx.translation.save-started' ).fire();
-
- if ( saveRequest ) {
- saveRequest.abort();
- }
-
saveRequest = api.postWithToken( 'csrf', {
action: 'cxsave',
assert: 'user',
@@ -144,24 +154,35 @@
progress: JSON.stringify( mw.cx.getProgress() )
} ).done( function ( response ) {
self.onSaveComplete( sections, response.cxsave );
+
+ // Reset fail counter.
+ failCounter = 0;
+ clearTimeout( timer );
+ timer = null;
} ).fail( function ( errorCode, details ) {
+ var delay;
+
if ( details.exception !== 'abort' ) {
self.onSaveFailure( errorCode, details );
}
- } ).always( function () {
- saveRequest = null;
- if ( failCounter > 10 ) {
- // If there are more than 10 save errors, stop
autosave at timer triggers.
- // It will get restarted on further translation
edits.
+
+ failCounter++;
+ clearTimeout( timer );
+ timer = null;
+
+ if ( failCounter > 5 ) {
+ // If there are more than few errors, stop
autosave at timer triggers.
// Show a bigger error message at this point.
mw.hook( 'mw.cx.error' ).fire( mw.msg(
'cx-save-draft-error' ) );
- return;
+ // This will allow any change to trigger save
again
+ failCounter = 0;
+ } else {
+ // Delay in seconds, failCounter is [1,5]
+ delay = 60 * failCounter;
+ timer = setTimeout( self.save.bind( self,
'retry' ), delay * 1000 );
}
- // Irrespective of success or fail, schedule next
autosave
- clearTimeout( timer );
- timer = setTimeout( function () {
- self.save();
- }, 5 * 60 * 1000 );
+ } ).always( function () {
+ saveRequest = null;
} );
};
@@ -197,9 +218,6 @@
mw.cx.sourceTitle,
mw.cx.targetTitle
);
-
- // Reset fail counter.
- failCounter = 0;
};
ContentTranslationStorage.prototype.onSaveFailure = function (
errorCode, details ) {
@@ -219,7 +237,6 @@
mw.cx.targetTitle,
JSON.stringify( details )
);
- failCounter++;
};
ContentTranslationStorage.prototype.markForSave = function (
$targetSection ) {
--
To view, visit https://gerrit.wikimedia.org/r/350200
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I37d8de3969700aa2013b74f54b976349632094ae
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/ContentTranslation
Gerrit-Branch: wmf/1.29.0-wmf.20
Gerrit-Owner: KartikMistry <[email protected]>
Gerrit-Reviewer: Dereckson <[email protected]>
Gerrit-Reviewer: KartikMistry <[email protected]>
Gerrit-Reviewer: Nikerabbit <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
