Issue #1364


gofer sets __debug__ flag in python interpreter, resulting in yum exception

Added by cduryee about 7 years ago. Updated over 3 years ago.

Start date:
Due date:
Estimated time:
2. Medium
Platform Release:
Sprint Candidate:
Pulp 2


The gofer client makes various calls to yum in order to install packages. It looks like some of the yum code checks for "__debug__" and will raise an exception if the rpm db checksum is invalid. Note that the db index gets cleared before this happens; the error is to aid in debugging. Ideally, the exception will not get raised since it is a "benign" error that is already being handled.

To fix this, I changed the following in /usr/bin/goferd:

diff --git a/bin/gofer b/bin/gofer
index ddb9e9e..033f591 100755
--- a/bin/gofer
+++ b/bin/gofer
@@ -1,4 +1,4 @@
-#! /usr/bin/env python
+#! /usr/bin/python -O
 # Copyright (c) 2015 Red Hat, Inc.

To reproduce this issue, you will need to attempt to apply >100 (possibly >200 errata) to a system. More details are in the linked BZ.

Actions #1

Updated by mhrivnak about 7 years ago

  • Priority changed from Normal to High
  • Triaged changed from No to Yes

Let's figure out how to get the optimization on for gofer, likely by setting the environment variable PYTHONOPTIMIZE

Actions #3

Updated by semyers about 7 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to semyers
Actions #4

Updated by semyers about 7 years ago

  • Priority changed from High to Urgent
Actions #5

Updated by semyers about 7 years ago

I was able to reproduce this, which gives me some leads on solving it, but what I'm coming up with isn't really great.

First, the traceback:

Traceback (most recent call last):
  File /usr/lib/python2.7/site-packages/pulp/agent/lib/, line 61, in install
    _report = handler.install(conduit, units, dict(options))
  File /usr/lib/python2.7/site-packages/pulp_rpm/handlers/, line 100, in install
    details = pkg.install(names)
  File /usr/lib/python2.7/site-packages/pulp_rpm/handlers/, line 159, in install
  File /usr/lib/python2.7/site-packages/pulp_rpm/handlers/, line 604, in processTransaction
    YumBase.processTransaction(self, callback, rpmDisplay=display)
  File /usr/lib/python2.7/site-packages/yum/, line 6462, in processTransaction
    if numleft == 0:
  File /usr/lib/python2.7/site-packages/yum/, line 6574, in _doTransaction
    if rpmlib_only:
  File /usr/lib/python2.7/site-packages/yum/, line 1879, in runTransaction
    raise Errors.YumRPMTransError(msg=_(Could not run transaction.),
  File /usr/lib/python2.7/site-packages/yum/, line 384, in dropCachedDataPostTransaction
  File /usr/lib/python2.7/site-packages/yum/, line 778, in _deal_with_bad_rpmdbcache
    raise Errors.PackageSackError, 'Rpmdb checksum is invalid: %s' % caller
PackageSackError: Rpmdb checksum is invalid: dCDPT(pkg checksums): NetworkManager.x86_64 1:1.0.0-14.git20150121.b4ea599c.el7

Here's what _deal_with_bad_rpmdbcache looks like:;a=blob;f=yum/;hb=HEAD#l758

The exception that gets thrown in debug, Errors.PackageSackError, is our problem. debug is a constant. Its value is decided when the python interpreter starts up, and (because it's a constant) attempting to change its value inside that interpreter at best does nothing, and at worst raises a SyntaxError. So, setting debug to False before the code runs is out, as an option.

The first option for dealing with it, turning on optimization, is pretty easy, but we don't really know what problems that might cause. We're reasonably certain that pulp and gofer would work reasonable well with optimization on, but it's difficult to say the same for every library we integrate with. I'm not comfortable with making the call that turning on optimization is the way to solve this, so for now I'll rule that out.

Another option is a little less invasive, but has its own share of drawbacks. As seen in the link above, it's a very simple method, and I think it would be easy (but a little evil) to patch it in-place, with a mechanism like this:

def _spoofed_deal_with_bad_rpmdbcache(package_sack, caller):
    # This is a modified version of the yum.RPMDBPackageSack method, with a troublesome
    # bit of code taken out, mainly because the comment on the code says it's okay.
    # That comment can be see here:
    # That code doesn't exist in newer
    from yum import misc
    misc.unlink_f(package_sack._cachedir + "/version")
    misc.unlink_f(package_sack._cachedir + '/conflicts')
    misc.unlink_f(package_sack._cachedir + '/obsoletes')
    misc.unlink_f(package_sack._cachedir + '/file-requires')
    misc.unlink_f(package_sack._cachedir + '/pkgtups-checksums')

def _spoof_deal_with_bad_rpmdbcache():
    from yum import rpmsack
    original_method = rpmsack.RPMDBPackageSack._deal_with_bad_rpmdbcache
    rpmsack.RPMDBPackageSack._deal_with_bad_rpmdbcache = _spoofed_deal_with_bad_rpmdbcache
    rpmsack.RPMDBPackageSack._deal_with_bad_rpmdbcache = original_method

Then we can wrap the processTransaction call as rpmtools:604 (as seen in the traceback) with the spoofing context manager, for a sort of surgical strike that removes the offending debug code. This is clearly Bad, because there's no guarantee we won't eventually diverge from the upstream yum code (though we can check for the divergence in our own testing). With the advent of dnf, I'm not sure how much of a problem that is. I also don't know if an upstream issue has been created for this, mostly because I don't know where upstream issues for yum get made, but I'm reticent to monkey-patch around an upstream bug without actually having an upstream bug to point to when I do it.

...or we could just turn optimization on for goferd and see what, if anything, explodes...

Speaking of dnf: While the code in question is present in the current yum master commit, the code does not exist in dnf, and will therefore not fail in this way if we used the dnf libs instead of yum. That's a bit out of scope for this issue, though. :)

Actions #6

Updated by semyers about 7 years ago

  • Status changed from ASSIGNED to POST

After a few good discussions with the team, and weighing of options, we're going the route of flipping on python optimizations for goferd, because reasons:

  1. It's a generally good idea, not to be feared
  2. Doing so didn't break any tests
  3. Manual testing of this issue reveals that it is fixed when goferd runs with optimizations enabled
  4. Spot-checking our code and dependencies revealed no instances of the deadly "try: assert/except AssertionError" pattern that would be thwarted (and rightly so) by enable optimizations.

In the extremely unlikely event that this causes more problems than it fixes, those problems are new and exciting bugs that we should file and fix. I'll be tackling the issue of getting this guy filed upstream and reporting back.

Actions #7

Updated by mhrivnak about 7 years ago

  • Status changed from POST to MODIFIED
Actions #8

Updated by about 7 years ago

Tagged upstream as gofer 2.6.7-1.

Actions #9

Updated by rbarlow almost 7 years ago

  • Status changed from MODIFIED to 5
  • Platform Release set to 2.8.0
Actions #10

Updated by over 6 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE
Actions #12

Updated by bmbouter over 3 years ago

  • Tags Pulp 2 added

Also available in: Atom PDF