[go: nahoru, domu]
Page MenuHomePhabricator
Create Task
Maniphest T274179

Deprecate and remove UserIdentity::getActorId()
Closed, ResolvedPublic
Actions

Description

While intuitively UserIdentity::getActorId() seems useful to have, there is no way to know at which point an ID will be assigned to an actor. Any code calling the getter would have to implement a fallback for the case no actor ID is present yet. And any code creating a UserIdentity value object would have to check if an ID has been assigned yet.

For this reason, the actor ID should be treated as an optimization/normalization internal to the storage layer, and not modeled on the level of domain entities (the "user"). Application logic should not know or care about actor IDs. Storage level code should rely on the ActorNormalization service to obtain actor IDs when needed for database operations.

  • soft-deprecate getActorId()
  • remove known callers of getActorId()
  • hard-deprecate getActorId()
  • wait for at least one release branch and three months per the stable interface policy
  • remove getActorId()

Details

Show related patches Customize query in gerrit

Related Objects
Search...

Event Timeline

daniel renamed this task from Deprecate UserIdentity::getActorId() to Deprecate and remove UserIdentity::getActorId() .Feb 8 2021, 7:35 PM
daniel triaged this task as Medium priority.
daniel created this task.
daniel updated the task description. (Show Details)
daniel moved this task from Unsorted pile to UserStore pile on the Platform Team Workboards (MW Expedition) board.
• Pchelolo merged a task: T274148: Remove UserIdentity::getActorId method.Feb 8 2021, 7:58 PM
• Pchelolo subscribed.
daniel claimed this task.Feb 15 2021, 6:59 PM
daniel raised the priority of this task from Medium to High.
gerritbot added a comment.Feb 15 2021, 7:00 PM

Change 664325 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] EXPERIMENT: stop using getActorId

https://gerrit.wikimedia.org/r/664325

gerritbot added a project: Patch-For-Review.Feb 15 2021, 7:00 PM
gerritbot added a comment.Feb 20 2021, 10:08 PM

Change 665546 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Remove $actor field from UsererIdentityValue

https://gerrit.wikimedia.org/r/665546

• Pchelolo moved this task from UserStore pile to Doing on the Platform Team Workboards (MW Expedition) board.Feb 23 2021, 5:57 PM
gerritbot added a comment.Feb 26 2021, 11:37 AM

Change 667142 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Deprecate UserIdentity::getActorId()

https://gerrit.wikimedia.org/r/667142

gerritbot added a comment.Mar 1 2021, 10:18 PM

Change 664325 merged by jenkins-bot:
[mediawiki/core@master] ActorStore: introduce findActorIdByName

https://gerrit.wikimedia.org/r/664325

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.33; 2021-03-02).Mar 1 2021, 11:00 PM
daniel added a subtask: T276853: Convert RecentChange to UserIdentity and ActorStore.Mar 8 2021, 6:42 PM
daniel moved this task from Doing to Waiting for Review on the Platform Team Workboards (MW Expedition) board.Mar 9 2021, 6:22 PM
gerritbot added a comment.Mar 12 2021, 3:39 PM

Change 671181 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/extensions/AbuseFilter@master] UserIdentityValue: do not provide actor ID

https://gerrit.wikimedia.org/r/671181

gerritbot added a comment.Mar 13 2021, 9:30 PM

Change 667142 merged by jenkins-bot:
[mediawiki/core@master] Deprecate UserIdentity::getActorId()

https://gerrit.wikimedia.org/r/667142

gerritbot added a comment.Mar 13 2021, 9:30 PM

Change 671181 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] UserIdentityValue: do not provide actor ID

https://gerrit.wikimedia.org/r/671181

ReleaseTaggerBot edited projects, added MW-1.36-notes (1.36.0-wmf.35; 2021-03-16); removed MW-1.36-notes (1.36.0-wmf.33; 2021-03-02).Mar 13 2021, 10:00 PM
kostajh mentioned this in T276307: Expectation (masterConns <= 0) not met (via GrowthExperimentsLinkRecommendationStore constructor).Mar 17 2021, 9:39 PM
kostajh subscribed.Mar 30 2021, 7:37 AM

Is there an ETA on this? We have every week ~250,000 DBPerformance log messages due to the invalidateCache() call causing a connection to the main DB in this code:

       /**
	 * Sets the actor id.
	 *
	 * This method is deprecated upon introduction. It only exists for transition to ActorStore,
	 * and will be removed shortly - T274148
	 *
	 * @internal
	 * @deprecated since 1.36
	 * @param int $actorId
	 */
	public function setActorId( int $actorId ) {
		$this->mActorId = $actorId;
		$this->invalidateCache();
		$this->setItemLoaded( 'actor' );
	}

Stacktrace:

from /srv/mediawiki/php-1.36.0-wmf.36/includes/libs/rdbms/TransactionProfiler.php(430)
#0 /srv/mediawiki/php-1.36.0-wmf.36/includes/libs/rdbms/TransactionProfiler.php(212): Wikimedia\Rdbms\TransactionProfiler->reportExpectationViolated(string, string, integer)
#1 /srv/mediawiki/php-1.36.0-wmf.36/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1006): Wikimedia\Rdbms\TransactionProfiler->recordConnection(string, string, boolean)
#2 /srv/mediawiki/php-1.36.0-wmf.36/includes/libs/rdbms/loadbalancer/LoadBalancer.php(960): Wikimedia\Rdbms\LoadBalancer->getServerConnection(integer, string, integer)
#3 /srv/mediawiki/php-1.36.0-wmf.36/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1099): Wikimedia\Rdbms\LoadBalancer->getConnection(integer, array, string, integer)
#4 /srv/mediawiki/php-1.36.0-wmf.36/includes/user/User.php(2360): Wikimedia\Rdbms\LoadBalancer->getConnectionRef(integer)
#5 /srv/mediawiki/php-1.36.0-wmf.36/includes/user/User.php(2376): User->clearSharedCache(string)
#6 /srv/mediawiki/php-1.36.0-wmf.36/includes/user/User.php(2200): User->invalidateCache()
#7 /srv/mediawiki/php-1.36.0-wmf.36/includes/user/ActorStore.php(320): User->setActorId(integer)
#8 /srv/mediawiki/php-1.36.0-wmf.36/includes/user/ActorStore.php(367): MediaWiki\User\ActorStore->attachActorId(User, integer)
#9 /srv/mediawiki/php-1.36.0-wmf.36/includes/ActorMigration.php(453): MediaWiki\User\ActorStore->findActorId(User, Wikimedia\Rdbms\DBConnRef)
#10 /srv/mediawiki/php-1.36.0-wmf.36/includes/user/UserEditTracker.php(155): ActorMigration->getWhere(Wikimedia\Rdbms\DBConnRef, string, array)
#11 /srv/mediawiki/php-1.36.0-wmf.36/includes/user/UserEditTracker.php(138): MediaWiki\User\UserEditTracker->getUserEditTimestamp(User, integer)
#12 /srv/mediawiki/php-1.36.0-wmf.36/extensions/GrowthExperiments/includes/HomepageModules/Mentorship.php(69): MediaWiki\User\UserEditTracker->getLatestEditTimestamp(User)
#13 /srv/mediawiki/php-1.36.0-wmf.36/extensions/GrowthExperiments/includes/HelpPanelHooks.php(195): GrowthExperiments\HomepageModules\Mentorship::getMentorLastActive(User, User, RequestContext)
#14 /srv/mediawiki/php-1.36.0-wmf.36/extensions/GrowthExperiments/includes/HelpPanelHooks.php(131): GrowthExperiments\HelpPanelHooks::getMentorData(GlobalVarConfig, User, RequestContext)
#15 /srv/mediawiki/php-1.36.0-wmf.36/includes/HookContainer/HookContainer.php(330): GrowthExperiments\HelpPanelHooks::onBeforePageDisplay(OutputPage, SkinVector)
#16 /srv/mediawiki/php-1.36.0-wmf.36/includes/HookContainer/HookContainer.php(137): MediaWiki\HookContainer\HookContainer->callLegacyHook(string, array, array, array)
#17 /srv/mediawiki/php-1.36.0-wmf.36/includes/HookContainer/HookRunner.php(1008): MediaWiki\HookContainer\HookContainer->run(string, array, array)
#18 /srv/mediawiki/php-1.36.0-wmf.36/includes/OutputPage.php(2629): MediaWiki\HookContainer\HookRunner->onBeforePageDisplay(OutputPage, SkinVector)
#19 /srv/mediawiki/php-1.36.0-wmf.36/includes/MediaWiki.php(931): OutputPage->output(boolean)
#20 /srv/mediawiki/php-1.36.0-wmf.36/includes/MediaWiki.php(944): MediaWiki::{closure}()
#21 /srv/mediawiki/php-1.36.0-wmf.36/includes/MediaWiki.php(547): MediaWiki->main()
#22 /srv/mediawiki/php-1.36.0-wmf.36/index.php(53): MediaWiki->run()
#23 /srv/mediawiki/php-1.36.0-wmf.36/index.php(46): wfIndexMain()
#24 /srv/mediawiki/w/index.php(3): require(string)
#25 {main}
• Pchelolo closed subtask T276853: Convert RecentChange to UserIdentity and ActorStore as Resolved.Apr 6 2021, 4:54 PM
daniel added a project: MW-1.36-release.Apr 9 2021, 7:07 PM
gerritbot added a comment.Apr 14 2021, 2:40 PM

Change 665546 merged by jenkins-bot:

[mediawiki/core@master] Remove $actor field from UsererIdentityValue

https://gerrit.wikimedia.org/r/665546

Maintenance_bot removed a project: Patch-For-Review.Apr 14 2021, 3:10 PM
Jdforrester-WMF moved this task from Blocker to To deprecate or remove on the MW-1.36-release board.Apr 15 2021, 6:17 PM
gerritbot added a comment.Apr 15 2021, 6:43 PM

Change 679840 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@REL1_36] Remove $actor field from UsererIdentityValue

https://gerrit.wikimedia.org/r/679840

gerritbot added a project: Patch-For-Review.Apr 15 2021, 6:43 PM
gerritbot added a comment.Apr 15 2021, 9:32 PM

Change 679969 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/FileImporter@master] Remove actor arg from UserIdentityValue in FileRevisionFromRemoteUrl

https://gerrit.wikimedia.org/r/679969

gerritbot added a comment.Apr 15 2021, 10:52 PM

Change 679969 merged by jenkins-bot:

[mediawiki/extensions/FileImporter@master] Remove actor arg from UserIdentityValue in FileRevisionFromRemoteUrl

https://gerrit.wikimedia.org/r/679969

Umherirrender mentioned this in rEFLI24c0a2851794: Remove actor arg from UserIdentityValue in FileRevisionFromRemoteUrl.Apr 15 2021, 10:58 PM
ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.3; 2021-04-27).Apr 15 2021, 11:00 PM
Mainframe98 mentioned this in rSMIRdfb0e3b11f60: Do not create UserIdentityValue with actor id.Apr 19 2021, 8:33 AM
Mainframe98 mentioned this in rSMIR6d40d826d853: Do not create UserIdentityValue with actor id.Apr 19 2021, 9:08 AM
kostajh added a comment.Apr 20 2021, 11:47 AM
In T274179#6954753, @kostajh wrote:

Is there an ETA on this? We have every week ~250,000 DBPerformance log messages due to the invalidateCache() call causing a connection to the main DB in this code

@daniel & @Pchelolo could you let me know your thoughts on the above question please? If it's not going to be fixed any time soon, I can set up a filter for errors originating from this code in the Growth team dashboard.

gerritbot added a comment.Apr 20 2021, 11:56 AM

Change 681344 had a related patch set uploaded (by Ppchelko; author: Ppchelko):

[mediawiki/core@master] User::setActorId only invalidate cache if required

https://gerrit.wikimedia.org/r/681344

• Pchelolo added a comment.Apr 20 2021, 11:57 AM

The patch above should help.

gerritbot added a comment.Apr 20 2021, 7:46 PM

Change 681344 merged by jenkins-bot:

[mediawiki/core@master] User::setActorId only invalidate cache if required

https://gerrit.wikimedia.org/r/681344

• Pchelolo added a comment.Apr 20 2021, 7:48 PM
In T274179#7019659, @kostajh wrote:
In T274179#6954753, @kostajh wrote:

Is there an ETA on this? We have every week ~250,000 DBPerformance log messages due to the invalidateCache() call causing a connection to the main DB in this code

@daniel & @Pchelolo could you let me know your thoughts on the above question please? If it's not going to be fixed any time soon, I can set up a filter for errors originating from this code in the Growth team dashboard.

The time is now :) The patch above should in theory make this error either disappear or get to a considerably lower rates. We can backport it if it's urgent, otherwise it should show up in the next train.

kostajh added a comment.Apr 21 2021, 8:19 AM
In T274179#7022081, @Pchelolo wrote:
In T274179#7019659, @kostajh wrote:
In T274179#6954753, @kostajh wrote:

Is there an ETA on this? We have every week ~250,000 DBPerformance log messages due to the invalidateCache() call causing a connection to the main DB in this code

@daniel & @Pchelolo could you let me know your thoughts on the above question please? If it's not going to be fixed any time soon, I can set up a filter for errors originating from this code in the Growth team dashboard.

The time is now :) The patch above should in theory make this error either disappear or get to a considerably lower rates. We can backport it if it's urgent, otherwise it should show up in the next train.

Thank you for that! I think it's OK to leave it for the next train.

Jdforrester-WMF removed a project: MW-1.36-release.Apr 21 2021, 6:38 PM
gerritbot added a comment.Apr 26 2021, 4:05 PM

Change 679840 merged by jenkins-bot:

[mediawiki/core@REL1_36] Remove $actor field from UsererIdentityValue

https://gerrit.wikimedia.org/r/679840

Maintenance_bot removed a project: Patch-For-Review.Apr 26 2021, 4:11 PM
JJMC89 moved this task from Backlog to Other on the MediaWiki-User-management board.May 3 2021, 3:02 PM
gerritbot added a comment.May 11 2021, 6:45 PM

Change 689199 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/GoogleLogin@master] Remove actor arg from UserIdentityValue in FileRevisionFromRemoteUrl

https://gerrit.wikimedia.org/r/689199

gerritbot added a project: Patch-For-Review.May 11 2021, 6:45 PM
gerritbot added a comment.May 14 2021, 11:06 PM

Change 689199 merged by Umherirrender:

[mediawiki/extensions/GoogleLogin@master] Remove actor arg from UserIdentityValue in tests

https://gerrit.wikimedia.org/r/689199

gerritbot added a comment.May 14 2021, 11:08 PM

Change 691314 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/GoogleLogin@REL1_36] Remove actor arg from UserIdentityValue in tests

https://gerrit.wikimedia.org/r/691314

gerritbot added a comment.May 14 2021, 11:08 PM

Change 691314 merged by Umherirrender:

[mediawiki/extensions/GoogleLogin@REL1_36] Remove actor arg from UserIdentityValue in tests

https://gerrit.wikimedia.org/r/691314

• Pchelolo closed this task as Resolved.Jun 25 2021, 5:03 PM
gerritbot added a comment.Jun 21 2022, 9:08 PM

Change 807222 had a related patch set uploaded (by Zabe; author: Zabe):

[mediawiki/core@master] Drop support of passing an actor id to UserIdentityValue's constructor

https://gerrit.wikimedia.org/r/807222

Zabe subscribed.Jun 21 2022, 9:08 PM
gerritbot added a comment.Jun 21 2022, 9:12 PM

Change 807223 had a related patch set uploaded (by Zabe; author: Zabe):

[mediawiki/core@master] UserIdentityValue: Always throw a deprecation warning on passed actor id

https://gerrit.wikimedia.org/r/807223

gerritbot added a comment.May 20 2023, 8:36 AM

Change 807223 merged by jenkins-bot:

[mediawiki/core@master] UserIdentityValue: Always throw a deprecation warning on passed actor id

https://gerrit.wikimedia.org/r/807223

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.10; 2023-05-23).May 20 2023, 9:00 AM
gerritbot added a comment.Dec 27 2023, 8:24 PM

Change 807222 merged by jenkins-bot:

[mediawiki/core@master] Drop support of passing an actor id to UserIdentityValue's constructor

https://gerrit.wikimedia.org/r/807222

Maintenance_bot removed a project: Patch-For-Review.Dec 27 2023, 8:30 PM
ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.12; 2024-01-02).Dec 27 2023, 9:00 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits