Page MenuHomePhabricator
Create Task
Maniphest T234533

gerrit shows merged patch as pending
Closed, ResolvedPublic
Actions

Description

This patch: https://gerrit.wikimedia.org/r/c/operations/puppet/+/534389

  • was merged in early September:
commit 8bcb37652c245276d023720e3cac27609d8cd1b9
Author:     Manuel Arostegui <marostegui@wikimedia.org>
AuthorDate: Wed Sep 4 11:10:46 2019 +0200
Commit:     Marostegui <marostegui@wikimedia.org>
CommitDate: Thu Sep 5 05:12:03 2019 +0000

    realm.pp: Remove filejournal table from private list
    
    filejournal table has been dropped in production, remove it from the
    private table list.
    This requires sanitariums mysql restarts to pick up new filters
    
    Bug: T51195
    Change-Id: I97ddc30df17ceb84d563c0fc03a004692b1a3224
  • shows as pending in gerrit
  • is giving weird behavior when @Marostegui views it when logged in (serves a 404 very often)
  • PS2 shows with no contents, as it was 'rebased' upon production by @Marostegui recently (post-merge, when there's no difference between patch and production). PS1 shows with the 'proper' contents.

Related Objects

Event Timeline

CDanis created this task.Oct 3 2019, 3:03 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 3 2019, 3:03 PM
CDanis triaged this task as High priority.Oct 3 2019, 3:03 PM
thcipriani subscribed.Oct 3 2019, 3:20 PM

My question is: how was this merged? Gerrit doesn't seem to think it was merged through the gerrit interface.

I get a 404 on the page for https://gerrit.wikimedia.org/r/changes/operations%2Fpuppet~production~I97ddc30df17ceb84d563c0fc03a004692b1a3224/revisions/2/zuul~crd

This is the zuul dependsOn/neededBy/etc in the zuul plugin: https://gerrit.wikimedia.org/r/plugins/zuul/Documentation/rest-api-changes.md

Marostegui added a comment.Oct 3 2019, 3:26 PM
In T234533#5544167, @thcipriani wrote:

My question is: how was this merged? Gerrit doesn't seem to think it was merged through the gerrit interface.

I get a 404 on the page for https://gerrit.wikimedia.org/r/changes/operations%2Fpuppet~production~I97ddc30df17ceb84d563c0fc03a004692b1a3224/revisions/2/zuul~crd

This is the zuul dependsOn/neededBy/etc in the zuul plugin: https://gerrit.wikimedia.org/r/plugins/zuul/Documentation/rest-api-changes.md

It was merged the normal way, +2 and then puppet-merge on a puppet master (T51195#5467079)

thcipriani added a comment.Oct 3 2019, 3:32 PM

hrm, yes, I see this in the review notes as well:

commit 8bcb37652c245276d023720e3cac27609d8cd1b9
Author: Manuel Arostegui <marostegui@wikimedia.org>
Date:   Wed Sep 4 11:10:46 2019 +0200

    realm.pp: Remove filejournal table from private list
    
    filejournal table has been dropped in production, remove it from the
    private table list.
    This requires sanitariums mysql restarts to pick up new filters
    
    Bug: T51195
    Change-Id: I97ddc30df17ceb84d563c0fc03a004692b1a3224

Notes (review):
    Verified+2: jenkins-bot
    Code-Review+2: Marostegui <marostegui@wikimedia.org>
    Submitted-by: Marostegui <marostegui@wikimedia.org>
    Submitted-at: Thu, 05 Sep 2019 05:13:00 +0000
    Reviewed-on: https://gerrit.wikimedia.org/r/534392
    Project: operations/puppet
    Branch: refs/heads/production
Paladox subscribed.Oct 3 2019, 3:34 PM

The url is different in that @thcipriani (Reviewed-on: https://gerrit.wikimedia.org/r/534392) compared to the url in description https://gerrit.wikimedia.org/r/c/operations/puppet/+/534389

Marostegui added a comment.Oct 3 2019, 3:36 PM

The URL https://gerrit.wikimedia.org/r/c/operations/puppet/+/534389 is the one that just showed up on my Gerrit "Outgoing reviews", with that same patch that was merged a month ago.

thcipriani added a comment.Oct 3 2019, 3:37 PM
In T234533#5544222, @Paladox wrote:

The url is different in that @thcipriani (Reviewed-on: https://gerrit.wikimedia.org/r/534392) compared to the url in description https://gerrit.wikimedia.org/r/c/operations/puppet/+/534389

ah, good find. It has the same Change-Id in the commit though.

thcipriani added a comment.Oct 3 2019, 3:50 PM

OK. So this change was submitted twice.

The diffs are identical, as are the commit messages. My new question is: why didn't gerrit reject the change at 3:15?

$ git log -1 --format=%H refs/changes/89/534389/1
cc740c29c36f8d345d906c9f93550dd600c25b2c
$ git log -1 --format=%H refs/changes/92/534392/1
3ce5f8b8d3c400c526b21f4430004229855dc309
$ git cat-file -p cc740c29c36f8d345d906c9f93550dd600c25b2c
tree e2a6cc74fa7bc71ad1d46efc49be5bba5f91e116
parent bed13eb0a254ad46cd8c70309fbfc68960d7c3b3
author Manuel Arostegui <marostegui@wikimedia.org> 1567588246 +0200
committer Manuel Arostegui <marostegui@wikimedia.org> 1567588246 +0200

realm.pp: Remove filejournal table from private list

filejournal table has been dropped in production, remove it from the
private table list.
This requires sanitariums mysql restarts to pick up new filters

Bug: T51195
Change-Id: I97ddc30df17ceb84d563c0fc03a004692b1a3224
$ git cat-file -p 3ce5f8b8d3c400c526b21f4430004229855dc309
tree e2a6cc74fa7bc71ad1d46efc49be5bba5f91e116
parent bed13eb0a254ad46cd8c70309fbfc68960d7c3b3
author Manuel Arostegui <marostegui@wikimedia.org> 1567588246 +0200
committer Manuel Arostegui <marostegui@wikimedia.org> 1567588472 +0200

realm.pp: Remove filejournal table from private list

filejournal table has been dropped in production, remove it from the
private table list.
This requires sanitariums mysql restarts to pick up new filters

Bug: T51195
Change-Id: I97ddc30df17ceb84d563c0fc03a004692b1a3224

^ these commits are identical, their SHA1 is different because the commit date is different (3:12 vs 3:15)

thcipriani added a comment.Oct 3 2019, 3:50 PM
In T234533#5544228, @Marostegui wrote:

The URL https://gerrit.wikimedia.org/r/c/operations/puppet/+/534389 is the one that just showed up on my Gerrit "Outgoing reviews", with that same patch that was merged a month ago.

When you say "just showed up" do you mean it was not in your outgoing reviews until this morning?

Marostegui added a comment.Oct 3 2019, 3:51 PM

3:12 vs 3:15? I only hit a rebase today, I didn't merge anything

Marostegui added a comment.Oct 3 2019, 3:51 PM
In T234533#5544293, @thcipriani wrote:
In T234533#5544228, @Marostegui wrote:

The URL https://gerrit.wikimedia.org/r/c/operations/puppet/+/534389 is the one that just showed up on my Gerrit "Outgoing reviews", with that same patch that was merged a month ago.

When you say "just showed up" do you mean it was not in your outgoing reviews until this morning?

Yep, specifically until we restarted gerrit

thcipriani added a comment.Oct 3 2019, 4:34 PM

the change numbers should increase, i.e., 534389 was pushed up before 534392. This is supported by the git timestamps for the changes first being created in Gerrit:

  • 534389 - Wed Sep 4 09:12:09 2019 +0000
  • 534392 - Wed Sep 4 09:15:05 2019 +0000

My guess as to what happened:

  1. Initial push
  2. Gerrit creates 534389 on disk
  3. Push takes a while, Ctrl-C -> this cancels gerrit adding the change to index
  4. Second push, 534392 created, Gerrit indexes this change
  5. Time passes, 534392 gets merged
  6. Changes are reindexed from disk due to projects being deleted
  7. Gerrit restart reads from primary index, shows the change that has been on disk this whole time, but not in the 2ndary index

The fix is: we should just delete the unmerged change from disk. This will trigger a reindex.

thcipriani added a comment.Oct 3 2019, 4:36 PM
In T234533#5544444, @thcipriani wrote:

The fix is: we should just delete the unmerged change from disk. This will trigger a reindex.

Clarification: delete the unmerged change using the REST api to remove it and trigger a reindex.

thcipriani closed this task as Resolved.Oct 3 2019, 5:44 PM
thcipriani claimed this task.
In T234533#5544460, @thcipriani wrote:
In T234533#5544444, @thcipriani wrote:

The fix is: we should just delete the unmerged change from disk. This will trigger a reindex.

Clarification: delete the unmerged change using the REST api to remove it and trigger a reindex.

Deleted: https://gerrit.wikimedia.org/r/c/operations/puppet/+/534389

https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/534392/ remains

https://gerrit.wikimedia.org/r/changes/operations%2Fpuppet~production~I97ddc30df17ceb84d563c0fc03a004692b1a3224/revisions/2/zuul~crd no longer 404s.

Resolving.

Marostegui reopened this task as Open.Oct 22 2019, 5:25 AM
Marostegui added subscribers: Joe, Dzahn.

After the upgrade (T222391) the same behaviour as reported on this original ticket has showed up again with:
https://gerrit.wikimedia.org/r/#/c/operations/mediawiki-config/+/540006/

That was merged days ago as can be seen at T233698#5544010 but it now shows up on my "Outgoing reviews" section as not merged.

@Joe also reports that https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/534389 gives an error

Marostegui mentioned this in T236114: check and fix some Gerrit revs.Oct 22 2019, 7:33 AM
awight subscribed.EditedOct 22 2019, 7:49 AM

~~Same here, I found this merge/unmerged patch: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Popups/+/538641/

Worryingly, there's no record of the patch every having been CR+2, although it's merged to master.~~
Moved comment to T236114

hashar closed this task as Resolved.Oct 22 2019, 7:50 AM
hashar subscribed.
In T234533#5593695, @Marostegui wrote:

After the upgrade (T222391) the same behaviour as reported on this original ticket has showed up again with:
https://gerrit.wikimedia.org/r/#/c/operations/mediawiki-config/+/540006/

That was merged days ago as can be seen at T233698#5544010 but it now shows up on my "Outgoing reviews" section as not merged.

For some reason, some metadata from the old Gerrit server (cobalt) have been not been synced to the new server (gerrit1001), that is T236114. Some references will need to be synced. I reported your change on that task.

@Joe also reports that https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/534389 gives an error

That was exactly the subject of this T234533 and it has been deleted entirely (see above) in favor of keeping https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/534392

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