Skip to content

PLATFORM-9808 compare fork of 143 vs 139 #106

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

Closed
wants to merge 54 commits into from
Closed

Conversation

mmeller-wikia
Copy link

mszabo-wikia and others added 30 commits December 2, 2022 10:48
…t in DB

Workaround T193271 performance regression by letting extensions signal that a
particular message key cannot exist in the DB. This allows us to use a
MessageCache::get hook handler to dynamically handle problematic keys in one
place (MessageCachePerformance extension) without patching core, while
maintaining compatibility with conditional message lookups that trigger fallback
logic depending on the outcome of a Message::exists() existence check.
Hooks allows to modify cache options, before user is loaded. This can be used to pass additional `checkKeys`, which will refresh user if those changed.
…eExternalImage method

Currently `Parser` allows only hot linking images with ``.jpg|.jpeg|.png|.gif` in the end of url. As we use revision, version and query params in our Vignette urls, so they would not be allowed for render even if domain was added to `wgAllowExternalImagesFrom`. `ParserAllowExternalImage` hook will be used only if domain in image url is added to `wgAllowExternalImagesFrom` list or `wgAllowExternalImages` is set to `true`.
MediaWiki only supports 14 character timestamps, and most date input
fields accordingly limit the accepted input range to fit within that
constraint, so the largest acceptable date is 9999-12-31 23:59:59.
However, if an user's own timezone preference is set to a timezone with
a higher offset than the server timezone, such dates may overflow into
the year 10000 and cause an error ("The timestamp XYZ should have 14
characters"). This very commonly happens when an admin decides to block
an user until 9999-12-31 instead of using the infinite expiry for some
reason, effectively breaking the block log for every user with a
timezone offset higher than the server offset.

Making MediaWiki support dates beyond the year 10000 would be a larger
undertaking, so for now, limit the impact of this problem by ensuring
that userAdjust() does not generate a local date that sprintfDate()
would not be able to handle.
purgeCache() invalidates the global check key for MessageBlobStore,
which results in MessageBlobStore being emptied every minute due to
wiki creations or site-wide update.php runs. Disable it while we consult
with upstream on the best way forward.
Cherry picks from REL1_37-Fandom
These changes need to be backported to upstream.
PLATFORM-7823 Fix SiteStats warnings
Changes:
- List view as subpage of Special:EditWatchlist so this also
shows up as a tabs
- Use skin option to determine whether to output the links in
skins that do not support sub pages

Bug: T316818
Change-Id: I1cee39c72cd0a49f97c3bdf41e07a4748502b1e2
[Cherry-pick] Watchlist: Register existing sub menu as associated pages
Sorting of the original query is not very optimized and is very slow to run because is done by the rev_id which besides unique index don't have other indexes that would optimize this query. `revactor_rev` has proper index that allows sorting the index. Therefore, reversing the query makes on DB like Marvel the process ~6x quicker and is probably even more significant for larger Databases.

Original code result:
```
Completed merge of revision_actor into the revision table, 10000 rows updated.

real    1m18.585s
user    0m2.647s
sys     0m0.757s
```

Code with the reversed query:
```
Completed merge of revision_actor into the revision table, 10000 rows updated.

real    0m15.776s
user    0m2.228s
sys     0m1.153s
```
Reverse the query in migrateRevisionActorTemp.php
Rendering UserLinks or Tags, requires parsing which when run on thousands of records, adds up to significant amount of processing time. More often then not, this can be optimized by storing already visited tags or user links as those "renders" will not change for the single request run but are repeated constantly on the list.
UGC-4161 | Allow section edit links when no restrictions and user can…
For anonymous users, the user ID is always zero, so it is possible for cached user links to be reused for different anonymous users after baf5c0a. This is not what we want, so use the user name as the cache key instead, which should be unique for both anonymous and registered users.
MAIN-28662 | Vary RC user(talk) link cache on user name rather than ID
PLATFORM-8338 | Add additional caching to the ChangeList rendering
Bug: T312169
Change-Id: I4606b4214d40596a1959bb4c4d93fb78ca9d8265
When a user saves current filter settings, the "apply" button's
message toggles whether the filter is marked as "default" or not.
As per the ticket, the message should always remain the same.

The width of the popup also changed to be wider to accommodate
a slightly longer title and the toggle of the "apply" button
width with the default icon.

Bug: T217304
Change-Id: Iee76cb422cbb011e89dd69d2feb0e74b30b2452c
PLATFORM-8107 | Fix RC filter bookmark popup rendering
Comment on lines +895 to +910
/**
* Fandom change - start (@author ttomalak) - PLATFORM-8338
* Tags are repeated for most of the records, so during single eg. RecentChanges we
* should cache those that were already processed as doing that for each record takes
* significant amount of time.
*/
[ $tagSummary, $newClasses ] = $this->tagsCache->getWithSetCallback(
$rc->mAttribs['ts_tags'],
'changeslist',
$this->getContext()
fn() => ChangeTags::formatSummaryRow(
$rc->mAttribs['ts_tags'],
'changeslist',
$this->getContext()
)
);
/** Fandom change - end */

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

/**
* Tags are repeated for a lot of the records, so during single run of RecentChanges, we
* should cache those that were already processed as doing that for each record takes
* significant amount of time.
*/
[ $tagSummary, $newClasses ] = $this->tagsCache->getWithSetCallback(
$this->tagsCache->makeKey(
$rc->mAttribs['ts_tags'],
$this->getUser()->getName(),
$this->getLanguage()->getCode()
),
fn () => ChangeTags::formatSummaryRow(
$rc->mAttribs['ts_tags'],
'changeslist',
$this->getContext()
)
);
$classes = array_merge( $classes, $newClasses );
$s .= ' ' . $tagSummary;
}

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/936221/2/includes/changes/ChangesList.php

Comment on lines +38 to +48

/**
* Fandom change - start (@author ttomalak) - PLATFORM-8338
* Initialize cache to store processed user links
* @var MapCacheLRU
*/
private MapCacheLRU $userLinks;
/** @var MapCacheLRU */
private MapCacheLRU $userTalkLinks;
/** Fandom change - end */

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

private $linkRenderer;
/**
* @var MapCacheLRU
*/
private MapCacheLRU $userLinkCache;
/**
* @var MapCacheLRU
*/
private MapCacheLRU $toolLinkCache;

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/936221/2/includes/changes/RCCacheEntryFactory.php

Comment on lines +60 to +66
/**
* Fandom change - start (@author ttomalak) - PLATFORM-8338
* Initialize cache to store processed user links
*/
$this->userLinks = new MapCacheLRU( 50 );
$this->userTalkLinks = new MapCacheLRU( 50 );
/** Fandom change - end */
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

$this->context = $context;
$this->messages = $messages;
$this->linkRenderer = $linkRenderer;
$this->userLinkCache = new MapCacheLRU( 50 );
$this->toolLinkCache = new MapCacheLRU( 50 );
}

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/936221/2/includes/changes/RCCacheEntryFactory.php

Comment on lines +102 to +124
/**
* Fandom change - start (@author ttomalak) - PLATFORM-8338
* userToolLinks requires a lot of parser work to process multiple links that are
* rendered there, like contrib page, user talk, message wall etc. Often, active
* users will appear multiple times on same run of RecentChanges, and therefore it is
* unnecessary to process it for each RC record separately.
*/
$cacheEntry->usertalklink = $this->userTalkLinks->getWithSetCallback(
$cacheEntry->mAttribs['rc_user_text'],
// Should the contributions link be red if the user has no edits (using default)
false,
// Customisation flags (using default 0)
0,
// User edit count (using default )
null,
// do not wrap the message in parentheses
false
fn() => Linker::userToolLinks(
$cacheEntry->mAttribs['rc_user'],
$cacheEntry->mAttribs['rc_user_text'],
// Should the contributions link be red if the user has no edits (using default)
false,
// Customisation flags (using default 0)
0,
// User edit count (using default )
null,
// do not wrap the message in parentheses
false
)
);
/** Fandom change - end */
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

if ( !ChangesList::isDeleted( $cacheEntry, RevisionRecord::DELETED_USER ) ) {
/**
* userToolLinks requires a lot of parser work to process multiple links that are
* rendered there, like contrib page, user talk etc. Often, active
* users will appear multiple times on same run of RecentChanges, and therefore it is
* unnecessary to process it for each RC record separately.
*/
$cacheEntry->usertalklink = $this->toolLinkCache->getWithSetCallback(
$this->toolLinkCache->makeKey(
$cacheEntry->mAttribs['rc_user_text'],
$this->context->getUser()->getName(),
$this->context->getLanguage()->getCode()
),
static fn () => Linker::userToolLinks(
$cacheEntry->mAttribs['rc_user'],
$cacheEntry->mAttribs['rc_user_text'],
// Should the contributions link be red if the user has no edits (using default)
false,
// Customisation flags (using default 0)
0,
// User edit count (using default )
null,
// do not wrap the message in parentheses
false
)
);
}
return $cacheEntry;

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/936221/2/includes/changes/RCCacheEntryFactory.php

Comment on lines 319 to 335
} else {
$userLink = Linker::userLink(
$cacheEntry->mAttribs['rc_user'],
/**
* Fandom change - start (@author ttomalak) - PLATFORM-8338
* UserLink requires parser to render which when run on thousands of records can add
* up to significant amount of processing time.
* @see RCCacheEntryFactory::newFromRecentChange
*/
$userLink = $this->userLinks->getWithSetCallback(
$cacheEntry->mAttribs['rc_user_text'],
ExternalUserNames::getLocal( $cacheEntry->mAttribs['rc_user_text'] )
fn() => Linker::userLink(
$cacheEntry->mAttribs['rc_user'],
$cacheEntry->mAttribs['rc_user_text'],
ExternalUserNames::getLocal( $cacheEntry->mAttribs['rc_user_text'] )
)
);
/** Fandom change - end */
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

} else {
/**
* UserLink requires parser to render which when run on thousands of records can add
* up to significant amount of processing time.
* @see RCCacheEntryFactory::newFromRecentChange
*/
$userLink = $this->userLinkCache->getWithSetCallback(
$this->userLinkCache->makeKey(
$cacheEntry->mAttribs['rc_user_text'],
$this->context->getUser()->getName(),
$this->context->getLanguage()->getCode()
),
static fn () => Linker::userLink(
$cacheEntry->mAttribs['rc_user'],
$cacheEntry->mAttribs['rc_user_text'],
ExternalUserNames::getLocal( $cacheEntry->mAttribs['rc_user_text'] )
)
);
}
return $userLink;
}

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/936221/2/includes/changes/RCCacheEntryFactory.php

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments only - skipped

Comment on lines +1312 to +1315

if ( $subjectPage->isMainPage() ) {
array_unshift( $subjectMsg, 'nstab-mainpage' );
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

// * nstab-mediawiki
// * nstab-template
// * nstab-help
// * nstab-category
// * nstab-<subject namespace key>
$subjectMsg = [ "nstab-$subjectId" ];
if ( $subjectPage->isMainPage() ) {
array_unshift( $subjectMsg, 'nstab-mainpage' );
}
}

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/963708/2/includes/skins/SkinTemplate.php

@@ -50,6 +50,7 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
public const EDIT_CLEAR = 1;
public const EDIT_RAW = 2;
public const EDIT_NORMAL = 3;
public const VIEW = 4;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

public const EDIT_CLEAR = 1;
public const EDIT_RAW = 2;
public const EDIT_NORMAL = 3;
public const VIEW = 4;
/** @var string|null */
protected $successMessage;

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/16/includes/specials/SpecialEditWatchlist.php

Comment on lines +146 to +149
case self::VIEW:
$title = SpecialPage::getTitleFor( 'Watchlist' );
$out->redirect( $title->getLocalURL() );
break;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

switch ( $mode ) {
case self::VIEW:
$title = SpecialPage::getTitleFor( 'Watchlist' );
$out->redirect( $title->getLocalURL() );
break;
case self::EDIT_RAW:

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/16/includes/specials/SpecialEditWatchlist.php

Comment on lines +179 to +187
$skin = $this->getSkin();
// For legacy skins render the tabs in the subtitle
$subpageSubtitle = $skin->supportsMenu( 'associated-pages' ) ? '' :
' ' .
self::buildTools(
null,
$this->getLinkRenderer(),
$this->currentMode
);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

protected function outputSubtitle() {
$out = $this->getOutput();
$skin = $this->getSkin();
// For legacy skins render the tabs in the subtitle
$subpageSubtitle = $skin->supportsMenu( 'associated-pages' ) ? '' :
' ' .
self::buildTools(
null,
$this->getLinkRenderer(),
$this->currentMode
);
$out->addSubtitle(

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/16/includes/specials/SpecialEditWatchlist.php

Comment on lines +848 to +849
case 'view':
return self::VIEW;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

public static function getMode( $request, $par, $defaultValue = false ) {
$mode = strtolower( $request->getRawVal( 'action' ) ?? $par ?? '' );
switch ( $mode ) {
case 'view':
return self::VIEW;
case 'clear':
case self::EDIT_CLEAR:
return self::EDIT_CLEAR;

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/16/includes/specials/SpecialEditWatchlist.php

Comment on lines -187 to +216
$this->getLanguage(),
$this->getLinkRenderer(),
$this->currentMode
)
) . $subpageSubtitle
);
}

/**
* @inheritDoc
*/
public function getAssociatedNavigationLinks() {
return SpecialWatchlist::WATCHLIST_TAB_PATHS;
}

/**
* @inheritDoc
*/
public function getShortDescription( string $path = '' ): string {
return SpecialWatchlist::getShortDescriptionHelper( $this, $path );
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

// Empty string parameter can be removed when all messages
// are updated to not use $2
$this->msg( 'watchlistfor2', $this->getUser()->getName(), '' )->text()
) . $subpageSubtitle
);
}
/**
* @inheritDoc
*/
public function getAssociatedNavigationLinks() {
return SpecialWatchlist::WATCHLIST_TAB_PATHS;
}
/**
* @inheritDoc
*/
public function getShortDescription( string $path = '' ): string {
return SpecialWatchlist::getShortDescriptionHelper( $this, $path );
}
/**
* Executes an edit mode for the watchlist view, from which you can manage your watchlist
*/

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/16/includes/specials/SpecialEditWatchlist.php

Comment on lines +40 to +46
/** @var array */
public const WATCHLIST_TAB_PATHS = [
'Special:Watchlist',
'Special:EditWatchlist',
'Special:EditWatchlist/raw',
'Special:EditWatchlist/clear'
];
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

class SpecialWatchlist extends ChangesListSpecialPage {
public const WATCHLIST_TAB_PATHS = [
'Special:Watchlist',
'Special:EditWatchlist',
'Special:EditWatchlist/raw',
'Special:EditWatchlist/clear'
];

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/16/includes/specials/SpecialWatchlist.php

Comment on lines 164 to 167
'edit',
'raw',
'clear',
];
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

public function getSubpagesForPrefixSearch() {
return [
'edit',
'raw',
'clear',
];
}

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/16/includes/specials/SpecialWatchlist.php

Comment on lines +626 to +658
/**
* @inheritDoc
*/
public function getAssociatedNavigationLinks() {
return self::WATCHLIST_TAB_PATHS;
}

/**
* @param SpecialPage $specialPage
* @param string $path
* @return string
*/
public static function getShortDescriptionHelper( SpecialPage $specialPage, string $path = '' ): string {
switch ( $path ) {
case 'Watchlist':
return $specialPage->msg( 'watchlisttools-view' )->text();
case 'EditWatchlist':
return $specialPage->msg( 'watchlisttools-edit' )->text();
case 'EditWatchlist/raw':
return $specialPage->msg( 'watchlisttools-raw' )->text();
case 'EditWatchlist/clear':
return $specialPage->msg( 'watchlisttools-clear' )->text();
default:
return $path;
}
}

/**
* @inheritDoc
*/
public function getShortDescription( string $path = '' ): string {
return self::getShortDescriptionHelper( $this, $path );
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

/**
* @inheritDoc
*/
public function getAssociatedNavigationLinks() {
return self::WATCHLIST_TAB_PATHS;
}
/**
* @param SpecialPage $specialPage
* @param string $path
* @return string
*/
public static function getShortDescriptionHelper( SpecialPage $specialPage, string $path = '' ): string {
switch ( $path ) {
case 'Watchlist':
return $specialPage->msg( 'watchlisttools-view' )->text();
case 'EditWatchlist':
return $specialPage->msg( 'watchlisttools-edit' )->text();
case 'EditWatchlist/raw':
return $specialPage->msg( 'watchlisttools-raw' )->text();
case 'EditWatchlist/clear':
return $specialPage->msg( 'watchlisttools-clear' )->text();
default:
return $path;
}
}
/**
* @inheritDoc
*/
public function getShortDescription( string $path = '' ): string {
return self::getShortDescriptionHelper( $this, $path );
}

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/16/includes/specials/SpecialWatchlist.php

Comment on lines +669 to +677
$skin = $this->getSkin();
// For legacy skins render the tabs in the subtitle
$subpageSubtitle = $skin->supportsMenu( 'associated-pages' ) ? '' :
' ' .
SpecialEditWatchlist::buildTools(
null,
$this->getLinkRenderer(),
$this->currentMode
);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

public function doHeader( $opts, $numRows ) {
$user = $this->getUser();
$out = $this->getOutput();
$skin = $this->getSkin();
// For legacy skins render the tabs in the subtitle
$subpageSubtitle = $skin->supportsMenu( 'associated-pages' ) ? '' :
' ' .
SpecialEditWatchlist::buildTools(
null,
$this->getLinkRenderer(),
$this->currentMode
);
$out->addSubtitle(

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/16/includes/specials/SpecialWatchlist.php

$this->getLinkRenderer(),
$this->currentMode
)
) . $subpageSubtitle
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

// Empty string parameter can be removed when all messages
// are updated to not use $2
$this->msg( 'watchlistfor2', $this->getUser()->getName(), '' )->text()
) . $subpageSubtitle
);
$this->setTopText( $opts );

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/16/includes/specials/SpecialWatchlist.php

Comment on lines 8 to 17
}

> .oo-ui-popupWidget-popup > .oo-ui-popupWidget-head {
position: relative;
height: auto;
padding: 1em;
// Icon width + icon left position (rounded to 0.5 to give a little extra margin)
padding-right: 1.875em + 0.5em;

> .oo-ui-buttonWidget {
top: 0.5em;
}

> .oo-ui-iconWidget {
vertical-align: top;
}

> .oo-ui-labelElement-label {
float: none;
display: inline-block;
// Label doesn't wrap without `max-width`. First setting a pretty arbitrary percentage value.
max-width: 80%;
// Overwrite it with `calc` reduced by icon width and left margin combined.
max-width: calc( ~'100% - 1.42857143em - 0.25em' );
margin: 0 0 0 0.25em;
font-size: 1.2em;
font-weight: bold;
line-height: 1.25;
vertical-align: top;
}
}
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

@import 'mediawiki.skin.variables.less';
@import 'mediawiki.mixins.less';
.mw-rcfilters-ui-saveFiltersPopupButtonWidget {
&-popup {
&-layout,
&-options {
padding-bottom: 1.5em;
}
> .oo-ui-popupWidget-popup > .oo-ui-popupWidget-head {
> .oo-ui-labelElement-label {
font-size: 1.2em;
font-weight: bold;
line-height: 1.25;
}
}
}
}

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/883598/1/resources/src/mediawiki.rcfilters/styles/mw.rcfilters.ui.SaveFiltersPopupButtonWidget.less

Comment on lines +83 to +86
/** @var int|null Maximum seconds to wait on connection attempts */
protected $connectTimeout;
/** @var int|null Maximum seconds to wait on receiving query results */
protected $receiveTimeout;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

protected $serverName;
/** @var bool Whether this PHP instance is for a CLI script */
protected $cliMode;
/** @var int|null Maximum seconds to wait on connection attempts */
protected $connectTimeout;
/** @var int|null Maximum seconds to wait on receiving query results */
protected $receiveTimeout;
/** @var string Agent name for query profiling */
protected $agent;

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/868514/5/includes/libs/rdbms/database/Database.php

Comment on lines +252 to +253
$this->connectTimeout = $params['connectTimeout'] ?? null;
$this->receiveTimeout = $params['receiveTimeout'] ?? null;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - upstream:

$this->ssl = $params['ssl'] ?? (bool)( $flags & self::DBO_SSL );
$this->connectTimeout = $params['connectTimeout'] ?? null;
$this->receiveTimeout = $params['receiveTimeout'] ?? null;
$this->cliMode = (bool)$params['cliMode'];

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/868514/5/includes/libs/rdbms/database/Database.php

@@ -344,6 +350,7 @@ abstract protected function open( $server, $user, $password, $db, $schema, $tabl
* - flags : Optional bit field of DBO_* constants that define connection, protocol,
* buffering, and transaction behavior. It is STRONGLY advised to leave the DBO_DEFAULT
* flag in place UNLESS this database simply acts as a key/value store.
* - ssl : Whether to use TLS connections.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment only - skipped

Comment on lines +363 to +364
* - connectTimeout: Optional timeout, in seconds, for connection attempts.
* - receiveTimeout: Optional timeout, in seconds, for receiving query results.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment only - skipped

Comment on lines +51 to +60
$conds[] = 'revactor_rev >= ' . $dbw->addQuotes( $start );
}
while ( true ) {
$res = $dbw->newSelectQueryBuilder()
->select( [ 'rev_id', 'rev_actor', 'revactor_actor' ] )
->from( 'revision' )
->join( 'revision_actor_temp', null, 'rev_id=revactor_rev' )
->from( 'revision_actor_temp' )
->join( 'revision', null, 'rev_id=revactor_rev' )
->where( $conds )
->limit( $batchSize )
->orderBy( 'rev_id' )
->orderBy( 'revactor_rev' )
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK:

$start = (int)$this->getOption( 'start', 0 );
if ( $start > 0 ) {
$conds[] = $dbw->expr( 'revactor_rev', '>=', $start );
}
while ( true ) {
$res = $dbw->newSelectQueryBuilder()
->select( [ 'rev_id', 'rev_actor', 'revactor_actor' ] )
->from( 'revision_actor_temp' )
->join( 'revision', null, 'rev_id=revactor_rev' )
->where( $conds )
->limit( $batchSize )
->orderBy( 'revactor_rev' )
->caller( __METHOD__ )
->fetchResultSet();

@@ -89,7 +89,7 @@ protected function doDBUpdates() {

// @phan-suppress-next-line PhanTypeSuspiciousStringExpression last is not-null when used
$this->output( "... rev_id=$last, updated $updated\n" );
$conds = [ 'rev_id > ' . $dbw->addQuotes( $last ) ];
$conds = [ 'revactor_rev > ' . $dbw->addQuotes( $last ) ];
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK:

// @phan-suppress-next-line PhanTypeSuspiciousStringExpression last is not-null when used
$this->output( "... rev_id=$last, updated $updated\n" );
$conds = [ $dbw->expr( 'revactor_rev', '>', $last ) ];
// Sleep between batches for replication to catch up

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: We might remove this change, as we already migrated actor data

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should go with the upstream solution and take a look at it again if we encounter the same issues we were trying to resolve in the linked pr

@mmeller-wikia
Copy link
Author

Not needed anymore

@mmeller-wikia mmeller-wikia deleted the REL1_39-Fandom branch December 17, 2024 10:11
@mmeller-wikia mmeller-wikia restored the REL1_39-Fandom branch December 17, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.