| Issue 847: | Changeset erroneously MERGED, and DB error | |
| 5 people starred this issue and may be notified of changes. | Back to list |
Affected Version:
2.1.6.1
What steps will reproduce the problem?
Detailed report follows.
What is the expected output? What do you see instead?
A patch was reportedly submitted to master, and the accused submitter (an admin with Force-push-branch permission and who is prone to typos) says he didn't do it. The patch does not appear on origin/master. But the changeset in Gerrit says it is "(Merged)".
Please provide any additional information below.
Could be a race condition. Gerrit + Hudson + Users
Could be a database migration error. Upgraded to 2.1.6.1 early this month.
Could be a configuration problem. Error-prone admin and new admins.
Database reports "OrmDuplicateKeyException: patch_set_approvals"
Clues in order of time:
15:20 I reviewed changeset 136 via Gerrit. The first commit on this
patch did not have a Change-Id: in the comment because the git hook was
not yet installed (relatively new repo getting bootstrapped when this change
was first created). After the 3rd "new" changeset was created, I suggested
to the author that he might add the Change-Id to the next commit message. I
do not know if he followed this advice. I do not know if this advice was correct.
This is probably a red herring.
15:23:07 Hudson reports a build is starting because of a different changeset (137) on
a different project.
15:23:?? The Hudson Gerrit plugin failed to build, possibly because this changeset is
dependent on an abandoned changeset, or maybe because of a missing submodule ref
(gitlink) erroneously included on patchset 137. Hudson is new on this repo
and our gitlink policies are not hook-enforced yet. Regardless, Hudson
submitted a Verified:-1 update; it reported a build duration of 01:47,
making the end time 15:24:54 or so.
15:23:41 A database error in the gerrit logs appears during the Hudson build.
Detail below.
15:24:?? I reviewed changeset 137 via Gerrit. Hudson was testing this changeset
at the same time, coincidentally.
15:45 Error-prone admin posts a comment to the review and follows up with a push to
15:47 the wrong "magic branch" at 15:47. There is an anomalous message in the
now-Merged commit which reports the patch was submitted to fres/for/master.
"git ls-remote gerrit" does not show any branch with this name.
15:47:16 Gerrit change notification email is sent out reporting that the error-prone admin
has submitted his changeset, a charge he vehemently denies.
Notes: The score on this merged changeset is "V:0 R:-1"
======================================================================================================
Here is the error in the database log:
[2011-02-15 15:23:41,866] WARN /gerrit : Error in publishComments
com.google.gwtorm.client.OrmDuplicateKeyException: patch_set_approvals
at com.google.gwtorm.schema.sql.DialectPostgreSQL.convertError(DialectPostgreSQL.java:53)
at com.google.gwtorm.jdbc.JdbcAccess.convertError(JdbcAccess.java:331)
at com.google.gwtorm.jdbc.JdbcAccess.doInsert(JdbcAccess.java:178)
at com.google.gwtorm.jdbc.JdbcAccess.doInsert(JdbcAccess.java:35)
at com.google.gwtorm.client.impl.AbstractAccess.insert(AbstractAccess.java:56)
at com.google.gerrit.server.patch.PublishComments.publishApprovals(PublishComments.java:222)
at com.google.gerrit.server.patch.PublishComments.call(PublishComments.java:121)
at com.google.gerrit.server.patch.PublishComments.call(PublishComments.java:53)
at com.google.gerrit.httpd.rpc.Handler$1.call(Handler.java:53)
at com.google.gerrit.httpd.rpc.Handler.to(Handler.java:65)
at com.google.gerrit.httpd.rpc.patch.PatchDetailServiceImpl.publishComments(PatchDetailServiceImpl.java:132)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:616)
at com.google.gwtjsonrpc.server.MethodHandle.invoke(MethodHandle.java:91)
at com.google.gwtjsonrpc.server.JsonServlet.doService(JsonServlet.java:382)
at com.google.gwtjsonrpc.server.JsonServlet.service(JsonServlet.java:268)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
at com.google.inject.servlet.ServletDefinition.doService(ServletDefinition.java:216)
at com.google.inject.servlet.ServletDefinition.service(ServletDefinition.java:141)
at com.google.inject.servlet.ManagedServletPipeline.service(ManagedServletPipeline.java:93)
at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:63)
at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:134)
at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:59)
at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:134)
at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:59)
at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:134)
at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:59)
at com.google.gwtexpui.server.CacheControlFilter.doFilter(CacheControlFilter.java:76)
at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:129)
at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:59)
at com.google.gerrit.httpd.RequestCleanupFilter.doFilter(RequestCleanupFilter.java:54)
at com.google.inject.servlet.FilterDefinition.doFilter(FilterDefinition.java:129)
at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:59)
at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:122)
at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:110)
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1322)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:473)
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:921)
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:403)
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:856)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:117)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:114)
at org.eclipse.jetty.server.Server.handle(Server.java:352)
at org.eclipse.jetty.server.HttpConnection.handleRequest(HttpConnection.java:596)
at org.eclipse.jetty.server.HttpConnection$RequestHandler.content(HttpConnection.java:1069)
at org.eclipse.jetty.http.HttpParser.parseNext(HttpParser.java:805)
at org.eclipse.jetty.http.HttpParser.parseAvailable(HttpParser.java:218)
at org.eclipse.jetty.server.HttpConnection.handle(HttpConnection.java:426)
at org.eclipse.jetty.io.nio.SelectChannelEndPoint.handle(SelectChannelEndPoint.java:510)
at org.eclipse.jetty.io.nio.SelectChannelEndPoint.access$000(SelectChannelEndPoint.java:34)
at org.eclipse.jetty.io.nio.SelectChannelEndPoint$1.run(SelectChannelEndPoint.java:40)
at org.eclipse.jetty.util.thread.QueuedThreadPool$2.run(QueuedThreadPool.java:450)
at java.lang.Thread.run(Thread.java:636)
Caused by: java.sql.BatchUpdateException: Batch entry 0 INSERT INTO patch_set_approvals(value,granted,change_open,change_sort_key,change_id,patch_set_id,account_id,category_id)VALUES('0','2011-02-15 15:23:41.816000 -05:00:00','Y','001311a700000088','136','1','1000002','VRIF') was aborted. Call getNextException to see the cause.
at org.postgresql.jdbc2.AbstractJdbc2Statement$BatchResultHandler.handleError(AbstractJdbc2Statement.java:2569)
at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:1796)
at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:407)
at org.postgresql.jdbc2.AbstractJdbc2Statement.executeBatch(AbstractJdbc2Statement.java:2708)
at org.apache.commons.dbcp.DelegatingStatement.executeBatch(DelegatingStatement.java:297)
at org.apache.commons.dbcp.DelegatingStatement.executeBatch(DelegatingStatement.java:297)
at com.google.gwtorm.jdbc.JdbcAccess.execute(JdbcAccess.java:293)
at com.google.gwtorm.jdbc.JdbcAccess.doInsert(JdbcAccess.java:171)
... 52 more
Caused by: org.postgresql.util.PSQLException: ERROR: duplicate key value violates unique constraint "patch_set_approvals_pkey"
at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2062)
at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:1795)
... 58 more
======================================================================================================
Here's the notification email:
-------- Original Message --------
Subject: Change in pr1...tech[master]: added compile option to not use NAND in API Libs
Date: Tue, 15 Feb 2011 15:47:16 -0500
From: Foo Barr (Code Review) <gerrit@myco.com>
Reply-To: foo.barr@myco.com
CC: Phil Hord <hordp@myco.com>
Foo Barr has submitted this change and it was merged.
Change subject: added compile option to not use NAND in API Libs
......................................................................
added compile option to not use NAND in API Libs
Change-Id: Icac26c836966710e87895e70e306a333223de202
---
M apps/somepath/renamed-file.c
<deletia>
8 files changed, 41 insertions(+), 16 deletions(-)
Objections:
Phil Hord: I would prefer that you didn't submit this
--
Gerrit-MessageType: merged
Gerrit-Change-Id: Icac26c836966710e87895e70e306a333223de202
Gerrit-PatchSet: 1
Gerrit-Project: pr1/module/tech
Gerrit-Branch: master
Gerrit-Owner: Foo Barr <foo.barr@myco.com>
Gerrit-Reviewer: Foo Barr <foo.barr@myco.com>
Gerrit-Reviewer: Phil Hord <hordp@myco.com>
Gerrit-Reviewer: Steve Austin <sAustin@myco.com>
======================================================================================================
Here's the Gerrit review page for 133 (related to the above email):
Change Icac26c83: added compile option to not use NAND in API Libs
Change-Id: Icac26c836966710e87895e70e306a333223de202
Owner Foo Barr
Project pr1/module/tech
Branch master
Topic
Uploaded Feb 14, 2011 3:11 PM
Updated Feb 15, 2011 3:47 PM
Status Merged
added compile option to not use NAND in API Libs
Set Compile flag to define NO_NAND to turn this on
Changed pre Steve to #endif line see Gerrit
Removed #ifdef in NandBdmShim.c as it was not needed.
Moved define NO_NAND to sades.h
Change-Id: Icac26c836966710e87895e70e306a333223de202
Reviewer Verified Code Review
Foo Barr
Phil Hord -1 I would prefer that you didn't submit this
Steve Austin
Author Foo Barr<foo.barr@myco.com>Feb 11, 2011 1:46 PM
Committer Foo Barr<foo.barr@myco.com>Feb 14, 2011 11:35 AM
File Path Comments Size Diff Reviewed
Commit Message Side-by-Side Unified
M apps/mongo/makefile_common +2, -0 Side-by-Side Unified
<deletia>
Comments
Expand RecentExpand AllCollapse All
Phil Hord Patch Set 1: I would prefer that you didn't submit this (8 inline … 3:20 PM
Patch Set 1: I would prefer that you didn't submit this
(8 inline comments)
Whitespace noise issues.
I think if you add "Change-Id: Icac26c836966710e87895e70e306a333223de202" to your commit msg, you will get a patch instead of a new review...
Foo Barr Patch Set 1: (2 inline comments) I can fix these in my next commit or … 3:45 PM
Patch Set 1: (2 inline comments)
I can fix these in my next commit or squash them together. and resubmit.
Foo Barr Change has been successfully pushed into branch fres/for/master. 3:47 PM
Change has been successfully pushed into branch fres/for/master.
Press '?' to view keyboard shortcuts
Powered by Gerrit Code Review (2.1.6.1) | Report Bug
======================================================================================================
Here's the Gerrit review page for 136 (which the DB error refers to):
Change I6d46057d: Fritzed the whaznot, twice
Change-Id: I6d46057d9e9e8e31808344ba35df8b8cc5b618a1
Owner Foo Barr
Project pr1/super-project
Branch master
Topic
Uploaded Feb 15, 2011 3:23 PM
Updated Feb 15, 2011 4:04 PM
Status Abandoned
Fritzed the whaznot, twice
Change-Id: I6d46057d9e9e8e31808344ba35df8b8cc5b618a1
Reviewer Verified Code Review
Hudson CI Fails
Phil Hord -1 I would prefer that you didn't submit this
ID Subject Owner Project Branch Updated
Depends On
I71e43d86 ReAdded copy of commit-msg hook from scripts to submodules (ABANDONED) Foo Barr pr1/super-project master Feb 11
Needed By
(None)
Patch Set 1
4143821fcc2cd26dc7816eb6701d29f495bc2a8d (gitweb)
Author Foo Barr<foo.barr@myco.com>Feb 15, 2011 3:13 PM
Committer Foo Barr<foo.barr@myco.com>Feb 15, 2011 3:13 PM
Comments
Patch Set 1:
Build Started http://mongo.myco.com/hudson/job/gerrit-super-project/47/
Hudson CI Patch Set 1: Fails Build Failed FAILURE: … 3:23 PM
Patch Set 1: Fails
Build Failed
FAILURE: http://mongo.myco.com/hudson/job/gerrit-super-project/47/
Phil Hord Patch Set 1:
1. Depends on an abandoned change. 2. Do not commit gitlinks … 3:24 PM
Patch Set 1:
1. Depends on an abandoned change. 2. Do not commit gitlinks to the superproject.
Phil Hord Patch Set 1: I would prefer that you didn't submit this 3:41 PM
Patch Set 1: I would prefer that you didn't submit this
Foo Barr Patch Set 1: Abandoned gitlinks are not wanted :-( 4:04 PM
Patch Set 1: Abandoned
gitlinks are not wanted :-(
Press '?' to view keyboard shortcuts
Powered by Gerrit Code Review (2.1.6.1) | Report Bug
======================================================================================================
Here's the Gerrit review page for 137 (which the Hudson was testing at the time):
Change I6d46057d: s2diags and launchdir changes
Change-Id: I6d46057d9e9e8e31808344ba35df8b8cc5b618a1
Owner Foo Barr
Project pr1/super-project
Branch master
Topic
Uploaded Feb 15, 2011 3:23 PM
Updated Feb 15, 2011 4:04 PM
Status Abandoned
Change-Id: I6d46057d9e9e8e31808344ba35df8b8cc5b618a1
Reviewer Verified Code Review
Hudson CI Fails
Phil Hord -1 I would prefer that you didn't submit this
Add Reviewer
Included in
Dependencies
ID Subject Owner Project Branch Updated
Depends On
I71e43d86 ReAdded copy of commit-msg hook from scripts to submodules (ABANDONED) Foo Barr pr1/super-project master Feb 11
Needed By
(None)
Patch Set 1
4143821fcc2cd26dc7816eb6701d29f495bc2a8d (gitweb)
Author Foo Barr<foo.barr@myco.com>Feb 15, 2011 3:13 PM
Committer Foo Barr<foo.barr@myco.com>Feb 15, 2011 3:13 PM
ReviewRestore ChangeDiff All Side-by-SideDiff All Unified
File Path Comments Size Diff Reviewed
Commit Message Side-by-Side Unified
M makery +0, -0 Side-by-Side Unified
M tech +0, -0 Side-by-Side Unified
+0, -0
Comments
Expand RecentExpand AllCollapse All
Hudson CI Patch Set 1: Build Started … 3:23 PM
Patch Set 1:
Build Started http://mongo.myco.com/hudson/job/gerrit-super-project/47/
Hudson CI Patch Set 1: Fails Build Failed FAILURE: … 3:23 PM
Patch Set 1: Fails
Build Failed
FAILURE: http://mongo.myco.com/hudson/job/gerrit-super-project/47/
Phil Hord Patch Set 1: 1. Depends on an abandoned change. 2. Do not commit gitlinks … 3:24 PM
Patch Set 1:
1. Depends on an abandoned change. 2. Do not commit gitlinks to the superproject.
Phil Hord Patch Set 1: I would prefer that you didn't submit this 3:41 PM
Patch Set 1: I would prefer that you didn't submit this
Foo Barr Patch Set 1: Abandoned gitlinks are not wanted :-( 4:04 PM
Patch Set 1: Abandoned
gitlinks are not wanted :-(
Press '?' to view keyboard shortcuts
Powered by Gerrit Code Review (2.1.6.1) | Report Bug
Mar 5, 2011
#1
phil.hord
Nov 14, 2011
I've confirmed this is a duplicate of #635. I suspect the OrmDuplicateKey error is caused by two changesets having the same changeid and the DB not handling it. But the main problem is repeatable and is the same as bug #635 . Having said that, I see there has been some improvement on this problem and #635 has been closed. But the problem covered here still exists: # Developer wants a patch reviewed git push origin HEAD:refs/for/master # Developer (intentionally or not) pushes the same commit to a public branch git push origin HEAD:refs/heads/share/public # At this point the original patchset is marked as merged Should this be closed as a dup? I still want the original issue addressed (also covered in #635). Should #635 be re-opened, a new bug created, or this one cleaned up? Demo: { Push commit for review. Check status of our commit; see that it is "Open" Push commit to share/public branch Check status of our commit; see that it is "Merged" } echo foo >> foo && git add foo && git commit -mFoo [detached HEAD ac40f2b] Foo 1 files changed, 1 insertions(+), 0 deletions(-) CID=$(git log -1 | grep Change-Id: |cut -d: -f2) echo $CID I4ca9183e1225ed94e355b2f62b7e255952df2045 git push origin HEAD:refs/for/master Counting objects: 5, done. Delta compression using up to 2 threads. Compressing objects: 100% (2/2), done. Writing objects: 100% (3/3), 291 bytes, done. Total 3 (delta 1), reused 0 (delta 0) remote: Resolving deltas: 0% (0/1) remote: remote: New Changes: remote: https://gerrit.myco.com/gerrit/759 remote: To gerrit:testing * [new branch] HEAD -> refs/for/master git ssh gerrit gerrit query --current-patch-set ${CID} change I4ca9183e1225ed94e355b2f62b7e255952df2045 project: testing branch: master id: I4ca9183e1225ed94e355b2f62b7e255952df2045 number: 759 subject: Foo owner: name: Phil Hord email: hordp@cisco.com url: https://gerrit.myco.com/gerrit/759 lastUpdated: 2011-11-14 14:54:40 EST sortKey: 00190b8a000002f7 open: true status: NEW currentPatchSet: number: 1 revision: ac40f2b66323aabd8d62de9d758aadb79e051853 ref: refs/changes/59/759/1 uploader: name: Phil Hord email: hordp@cisco.com type: stats rowCount: 1 runTimeMilliseconds: 4 git push origin HEAD:refs/heads/share/public Total 0 (delta 0), reused 0 (delta 0) To gerrit:testing 25ea87d..ac40f2b HEAD -> share/public ssh gerrit gerrit query --current-patch-set ${CID} change I4ca9183e1225ed94e355b2f62b7e255952df2045 project: testing branch: master id: I4ca9183e1225ed94e355b2f62b7e255952df2045 number: 759 subject: Foo owner: name: Phil Hord email: hordp@cisco.com url: https://gerrit.myco.com/gerrit/759 lastUpdated: 2011-11-14 14:55:44 EST sortKey: 00190b8b000002f7 open: false status: MERGED currentPatchSet: number: 1 revision: ac40f2b66323aabd8d62de9d758aadb79e051853 ref: refs/changes/59/759/1 uploader: name: Phil Hord email: hordp@cisco.com type: stats rowCount: 1 runTimeMilliseconds: 2
Feb 17, 2012
This is a dup of #1142, but #1142 is more focused and being addressed.
Feb 6, 2014
Can also happen the other way round, at gerrit.wikimedia.org we had one patch which was marked open by the interface but had one depending change merged, because it was in fact merged. |
|
| ► Sign in to add a comment |