Ticket #778 (closed enhancement: fixed)

Opened 10 years ago

Last modified 10 years ago

Do not use dpkg-query in Xsession.d scripts

Reported by: broder Owned by: jdreed
Priority: normal Milestone: The Distant Future
Component: login chroot Keywords:
Cc: Fixed in version:
Upstream bug:

Description

Right now, several of the scripts in /etc/X11/Xsession.d use dpkg-query -W -f '${Status}' PACKAGE to test if a package is installed.

This is necessary because conffiles are not removed when a package is uninstalled (only when it's purged), and Debian Policy dictates that a conffile cause no change when the package is uninstalled (it's only kept so that the configuration can be restored should the package be installed in the future).

However, using dpkg-query requires loading the entire dpkg database into buffer cache, which is a slow process, at least the first time, and happens at a point where there's already lots of disk I/O happening. Looking at bootcharts, it generally takes about 2 seconds. Because that query has to finish before the script can determine whether to do anything, it blocks the login time.

Instead of using dpkg-query, all of those scripts should be changed to use a finer-grained test. For instance, the debathena-larvnet script could check for [ -x /usr/lib/debathena-larvnet/larvnet-wrapper ], which is much less filesystem-intensive.

Change History

comment:1 Changed 10 years ago by jdreed

  • Status changed from new to accepted
  • Owner set to jdreed

Changed in r24968 for debathena-larvnet, which already tests for larvnet-wrapper before wrapping the session in it. The dpkg-query test was added as part of the fix for #348, but if larvnet-wrapper is somehow sticking around after the package has been uninstalled, then dpkg is broken.

comment:2 Changed 10 years ago by jdreed

This is a big issue in -xsession, which does a lot of whining at the user via zenity. There's no real executable to test for as in the larvnet-wrapper example. So that leaves us with a couple of options:

  • Test for /usr/share/doc/debathena-xsession/changelog.gz, which is apparently cringeworthy.
  • Can the Xsession.d scripts in turn test for and source a file in /usr/share/whatever which contains the _actual_ content?

comment:3 Changed 10 years ago by jdreed

Can the Xsession.d scripts in turn test for and source a file in /usr/share/whatever which contains the _actual_ content?

Looks like they can, and that's probably the right way to do this. So that means twice as many files for debathena-xsession, but it'll make logins slightly faster.

comment:4 Changed 10 years ago by jdreed

Fixed in r24969 for -bugme

Of course, the -xsession fix doesn't help us for -gconf2-config or -desktop-config. So we need to:

  • find a way to designate these as not conffiles
  • ignore policy and remove them in the maintainer scripts
  • suck it up and test for /usr/share/doc/<package>/changelog.gz

comment:5 Changed 10 years ago by jdreed

It was noted on zephyr that we can avoid testing for the Debian-specific /usr/share/doc stuff by shipping a /usr/share/<package>/is-installed file and testing for that.

comment:6 Changed 10 years ago by kaduk

It was noted on zephyr that sourcing a broken symlink would cause scripts that use set -e to fail. Wouldn't appending '
true' fix that?

comment:7 Changed 10 years ago by geofft

Hm? The vague proposal was whether we could just symlink to a non-conffile xsession.d script, in the hopes that uninstalling the package would leave a broken symlink that would be ignored, but the thing that sources it is /etc/X11/xsession (which we don't touch and absolutely don't want to). Adding || true would mask a bunch of errors, also.

Anyway, we decided not to go the broken-symlink route.

comment:8 Changed 10 years ago by jdreed

For gconf2-config and -desktop-config, I plan on going with the "ship a file and test for it" approach unless we have a better way.

comment:9 Changed 10 years ago by jdreed

Fixed in r24976 for -xsession. We don't need the test in 98debathena-xsession, since we can test for the relevant files in /usr/lib/init. Everything else can't test for its resources (because a test for , e.g., zenity or AFS, is pointless), and so gets sourced from a file in /usr/share if it exists. I initially thought we could skip 00debathena-gdm-sucks or 05debathena-nocalls, but GDMSESSION could be cached in dmrc or something stupid.

One hopes that whatever time penalty we take for sourcing twice the number of files is still less then the penalty for running dpkg-query (which I think is true, especially on older cluster machines)

comment:10 Changed 10 years ago by jdreed

  • Status changed from accepted to committed

Fixed r24977-r24978 for gconf2-config.

Before I actually build this, just to confirm, is it just dpkg-query that makes us lose? Or any dpkg invocation? Could we do, e.g.

dpkg -l debathena-xsession | grep -q ^ii\

comment:11 Changed 10 years ago by amu

That's a fair question. dpkg -l is just historical syntax for dpkg-query -l, predating dpkg-query's availability (mid-2002) as a separate program. Either way, it will potentially be slow, so shipping extra files (serving either as simple flags or as scripts to source if present) is indeed the way to go for best performance.

comment:12 Changed 10 years ago by jdreed

  • Status changed from committed to development

TTL expired. Built and uploaded gconf-config 1.8.1, larvnet 10.0.7, bugme 10.0.3-0debathena1, and xsession 1.17. Testing required.

It's probably worth noting that "machtype -L" is pronounced "dpkg-query" these days, so we may want yet another ticket to tackle that.

comment:13 Changed 10 years ago by jdreed

  • Status changed from development to proposed

comment:14 Changed 10 years ago by jdreed

  • Status changed from proposed to closed
  • Resolution set to fixed
Note: See TracTickets for help on using tickets.