-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On 04/17/2015 12:56 AM, Carl Baldwin wrote: > Stephen, > > On Thu, Apr 16, 2015 at 4:21 PM, Ma, Stephen B. <stephen...@hp.com> > wrote: >> As it stands currently, neutron on the stable/juno branch behaves >> as if https://review.openstack.org/#/c/164329/ was never merged. >> The merge of patch https://review.openstack.org/#/c/153181 puts >> some LOG.debug statements in the for-loop of the same function. >> The net result is the unintentional revert of patch >> https://review.openstack.org/#/c/164329/. > > I wouldn't call this a revert. Instead, it brought back some > context that the original patch dealt with but that the cherry pick > did not. This is a regression but not a revert of the patch. > >> According to review.openstack.org, the patch >> https://review.openstack.org/#/c/153181 merged on April 1st at >> 1:56 pm. Patch https://review.openstack.org/#/c/164329/ merged on >> the same day at 2:21 pm. After 153181 merged, the review of >> 164329 should have triggered a merge conflict error which would >> have compelled the owner to do a rebase. The merge conflict >> wasn’t reported and the patches merged anyway. Does this point >> to a problem with the Jenkins CI system? > > This was not a problem with the CI system. The infrastructure > behaved exactly as it should. > > The problem happened when [4] was proposed with a lesser scope > than [2] and then [3] merged making that extra scope relevant > again. I understand why the scope was reduced. It is often > necessary to react to changes in context when cherry picking. That > is the nature of cherry picking, bringing a patch in to a new > context and reacting. > > It was unfortunate that [3] brought back the relevant context > which made the broader scope of the change in [2] necessary and > then they merged together. > > After having given it some thought, I have come to the conclusion > that the only way to catch this would have been to include a proper > test with [2] and [4] that log debug statements are not executed in > the loop. Such a test would have kicked out whichever of [3] and > [4] happened to merge last from the gate. It would've been really > annoying to fail in the gate but it would have flagged the problem > then and there. Really, the failure was in not requiring the > tests necessary to prevent regression. Until we add these tests, > we will be vulnerable to this regression again. For example, I > could merge a new patch to add more logging to this loop today and > we'd be back to square one. > > Carl > > [1] https://review.openstack.org/#/c/147455 [2] > https://review.openstack.org/#/c/149784 [3] > https://review.openstack.org/#/c/153181 [4] > https://review.openstack.org/#/c/164329 >
Hi Carl, thanks for analysis. I've sent a regression unit test for master. Once it's merged in master, I'll propose for stable till Juno. [1]: https://review.openstack.org/175445 /Ihar -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJVNReAAAoJEC5aWaUY1u573MgIAK03+RqvujHgtL9SmyjXBDtn K2awpRWCHM4cmb6Hr6MLVbWNPEC2svjOHWOSRfRZ5vw05xk/ViJ/UM17b7Jz9Drf cTMVJ6G7lzt+XaCQQeB72NbZSEdEfFAcLLNnb9DzVxRjRcvLcu0dV/tpgJ7YeuMn vGwAUJkv+5af6iTy0BmDpJZh2hZ9OFXnesLxCcHY4VKcCX7HiXwhSEjhF1pCOdPd SwRC3MgqL5gcLchoYzcNZ2DtTwK881LixLIh4VhY14VTKzwF1zVJBbar2V5N77RG 9cukeTSsPg+oft/M1JclWUQrQ4G0JWZgE8B5VibpYnF45rYYWDlvAVWwa1pv6uQ= =KgGO -----END PGP SIGNATURE----- __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev