[gentoo-portage-dev] [PATCH] portage: HTTP if-modified-since and compression

[gentoo-portage-dev] [PATCH] portage: HTTP if-modified-since and compression

[Mark Kubacki]

[-- Attachment #1: Type: text/plain, Size: 1284 bytes --]

Hi Portage devs,

The attached patches fix some issues I've noticed as maintainer and
user of Gentoo binhost(s). They're made against master/HEAD and can
easily be backported to 2.1*.

The first patch enables Portage to skip downloading a remote index if
the local copy is recent enough. E.g., the remote index didn't change
between to "emerge"-runs. This is done by setting "If-Modified-Since"
request-header. The server responds with HTTP code 304 and Portage
doesn't even load a single byte of the (large) index file.

By the second patch Portage will download remote indices—which are
text-files after all—compressed, if the remote server supports that.
Although de-compression introduces a small latency, this will save
bandwidth and transmission time. If the index needs to be fetched at
all, that is (see the patch above).

An index' TIMESTAMP entry is set before the corresponding file gets
written. If the difference between TIMESTAMP and modification time
("mtime") is greater than or the times span one second, remote index
files will be loaded despite the "If-Modified-Since" header. This is
because TIMESTAMP of the local copy is compared with the remote index'
"mtime". The third patch fixes that by setting "mtime" = TIMESTAMP.

-- 
Mark

[-- Attachment #2: 0001-Use-If-Modified-Since-HTTP-header-and-avoid-download.patch --]
[-- Type: application/octet-stream, Size: 4868 bytes --]

From a52924ce950fd2efbf99959e4dd0452b5feb92da Mon Sep 17 00:00:00 2001
From: W-Mark Kubacki <wmark@hurrikane.de>
Date: Wed, 1 Aug 2012 19:49:34 +0200
Subject: [PATCH 1/3] Use If-Modified-Since HTTP-header and avoid downloading
 a remote index if the local copy is recent enough.

---
 pym/portage/dbapi/bintree.py |   24 +++++++++++++++++++++---
 pym/portage/util/_urlopen.py |   33 ++++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/pym/portage/dbapi/bintree.py b/pym/portage/dbapi/bintree.py
index 9527b07..16ae8ec 100644
--- a/pym/portage/dbapi/bintree.py
+++ b/pym/portage/dbapi/bintree.py
@@ -54,6 +54,11 @@ if sys.hexversion >= 0x3000000:
 else:
 	_unicode = unicode
 
+class UseCachedCopyOfRemoteIndex(Exception):
+	# If the local copy is recent enough
+	# then fetching the remote index can be skipped.
+	pass
+
 class bindbapi(fakedbapi):
 	_known_keys = frozenset(list(fakedbapi._known_keys) + \
 		["CHOST", "repository", "USE"])
@@ -852,6 +857,7 @@ class binarytree(object):
 				if e.errno != errno.ENOENT:
 					raise
 			local_timestamp = pkgindex.header.get("TIMESTAMP", None)
+			remote_timestamp = None
 			rmt_idx = self._new_pkgindex()
 			proc = None
 			tmp_filename = None
@@ -861,8 +867,13 @@ class binarytree(object):
 				# slash, so join manually...
 				url = base_url.rstrip("/") + "/Packages"
 				try:
-					f = _urlopen(url)
-				except IOError:
+					f = _urlopen(url, if_modified_since=local_timestamp)
+					if hasattr(f, 'headers') and f.headers.get('timestamp', ''):
+						remote_timestamp = f.headers.get('timestamp')
+				except IOError as err:
+					if hasattr(err, 'code') and err.code == 304: # not modified (since local_timestamp)
+						raise UseCachedCopyOfRemoteIndex()
+
 					path = parsed_url.path.rstrip("/") + "/Packages"
 
 					if parsed_url.scheme == 'sftp':
@@ -903,7 +914,8 @@ class binarytree(object):
 					_encodings['repo.content'], errors='replace')
 				try:
 					rmt_idx.readHeader(f_dec)
-					remote_timestamp = rmt_idx.header.get("TIMESTAMP", None)
+					if not remote_timestamp: # in case it had not been read from HTTP header
+						remote_timestamp = rmt_idx.header.get("TIMESTAMP", None)
 					if not remote_timestamp:
 						# no timestamp in the header, something's wrong
 						pkgindex = None
@@ -931,6 +943,12 @@ class binarytree(object):
 						writemsg("\n\n!!! %s\n" % \
 							_("Timed out while closing connection to binhost"),
 							noiselevel=-1)
+			except UseCachedCopyOfRemoteIndex:
+				writemsg_stdout("\n")
+				writemsg_stdout(
+					colorize("GOOD", _("Local copy of remote index is up-to-date and will be used.")) + \
+					"\n")
+				rmt_idx = pkgindex
 			except EnvironmentError as e:
 				writemsg(_("\n\n!!! Error fetching binhost package" \
 					" info from '%s'\n") % _hide_url_passwd(base_url))
diff --git a/pym/portage/util/_urlopen.py b/pym/portage/util/_urlopen.py
index 307624b..a5db411 100644
--- a/pym/portage/util/_urlopen.py
+++ b/pym/portage/util/_urlopen.py
@@ -2,6 +2,9 @@
 # Distributed under the terms of the GNU General Public License v2
 
 import sys
+from datetime import datetime
+from time import mktime
+from email.utils import formatdate, parsedate
 
 try:
 	from urllib.request import urlopen as _urlopen
@@ -14,12 +17,26 @@ except ImportError:
 	import urllib2 as urllib_request
 	from urllib import splituser as urllib_parse_splituser
 
-def urlopen(url):
+# to account for the difference between TIMESTAMP of the index' contents
+#  and the file-'mtime'
+TIMESTAMP_TOLERANCE=5
+
+def urlopen(url, if_modified_since=None):
 	try:
-		return _urlopen(url)
+		request = urllib_request.Request(url)
+		request.add_header('User-Agent', 'Gentoo Portage')
+		if if_modified_since:
+			request.add_header('If-Modified-Since', _timestamp_to_http(if_modified_since))
+		opener = urllib_request.build_opener()
+		hdl = opener.open(request)
+		if hdl.headers.get('last-modified', ''):
+			hdl.headers.addheader('timestamp', _http_to_timestamp(hdl.headers.get('last-modified')))
+		return hdl
 	except SystemExit:
 		raise
-	except Exception:
+	except Exception as e:
+		if hasattr(e, 'code') and e.code == 304: # HTTPError 304: not modified
+			raise
 		if sys.hexversion < 0x3000000:
 			raise
 		parse_result = urllib_parse.urlparse(url)
@@ -40,3 +57,13 @@ def _new_urlopen(url):
 	auth_handler = urllib_request.HTTPBasicAuthHandler(password_manager)
 	opener = urllib_request.build_opener(auth_handler)
 	return opener.open(url)
+
+def _timestamp_to_http(timestamp):
+	dt = datetime.fromtimestamp(float(long(timestamp)+TIMESTAMP_TOLERANCE))
+	stamp = mktime(dt.timetuple())
+	return formatdate(timeval=stamp, localtime=False, usegmt=True)
+
+def _http_to_timestamp(http_datetime_string):
+	tuple = parsedate(http_datetime_string)
+	timestamp = mktime(tuple)
+	return str(long(timestamp))
-- 
1.7.8.6


[-- Attachment #3: 0002-Add-support-for-HTTP-compression-bzip2-gzip-and-defl.patch --]
[-- Type: application/octet-stream, Size: 2588 bytes --]

From 88a289b07642cb200b83b98f03d508dcbfd2ce64 Mon Sep 17 00:00:00 2001
From: W-Mark Kubacki <wmark@hurrikane.de>
Date: Wed, 1 Aug 2012 20:36:31 +0200
Subject: [PATCH 2/3] Add support for HTTP compression (bzip2, gzip and
 deflate).

---
 pym/portage/util/_urlopen.py |   32 +++++++++++++++++++++++++++++++-
 1 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/pym/portage/util/_urlopen.py b/pym/portage/util/_urlopen.py
index a5db411..70535c5 100644
--- a/pym/portage/util/_urlopen.py
+++ b/pym/portage/util/_urlopen.py
@@ -5,6 +5,7 @@ import sys
 from datetime import datetime
 from time import mktime
 from email.utils import formatdate, parsedate
+from StringIO import StringIO
 
 try:
 	from urllib.request import urlopen as _urlopen
@@ -27,7 +28,7 @@ def urlopen(url, if_modified_since=None):
 		request.add_header('User-Agent', 'Gentoo Portage')
 		if if_modified_since:
 			request.add_header('If-Modified-Since', _timestamp_to_http(if_modified_since))
-		opener = urllib_request.build_opener()
+		opener = urllib_request.build_opener(CompressedResponseProcessor)
 		hdl = opener.open(request)
 		if hdl.headers.get('last-modified', ''):
 			hdl.headers.addheader('timestamp', _http_to_timestamp(hdl.headers.get('last-modified')))
@@ -67,3 +68,32 @@ def _http_to_timestamp(http_datetime_string):
 	tuple = parsedate(http_datetime_string)
 	timestamp = mktime(tuple)
 	return str(long(timestamp))
+
+class CompressedResponseProcessor(urllib_request.BaseHandler):
+	# Handler for compressed responses.
+
+	def http_request(self, req):
+		req.add_header('Accept-Encoding', 'bzip2,gzip,deflate')
+		return req
+	https_request = http_request
+
+	def http_response(self, req, response):
+		decompressed = None
+		if response.headers.get('content-encoding') == 'bzip2':
+			import bz2
+			decompressed = StringIO.StringIO(bz2.decompress(response.read()))
+		elif response.headers.get('content-encoding') == 'gzip':
+			from gzip import GzipFile
+			decompressed = GzipFile(fileobj=StringIO(response.read()), mode='r')
+		elif response.headers.get('content-encoding') == 'deflate':
+			import zlib
+			try:
+				decompressed = StringIO.StringIO(zlib.decompress(response.read()))
+			except zlib.error: # they ignored RFC1950
+				decompressed = StringIO.StringIO(zlib.decompress(response.read(), -zlib.MAX_WBITS))
+		if decompressed:
+			old_response = response
+			response = urllib_request.addinfourl(decompressed, old_response.headers, old_response.url, old_response.code)
+			response.msg = old_response.msg
+		return response
+	https_response = http_response
-- 
1.7.8.6


[-- Attachment #4: 0003-Fix-index-file-s-mtime-which-can-differ-from-TIMESTA.patch --]
[-- Type: application/octet-stream, Size: 1545 bytes --]

From 2b7ba96c8c6e81541cfba095c113638ac9a847f4 Mon Sep 17 00:00:00 2001
From: W-Mark Kubacki <wmark@hurrikane.de>
Date: Wed, 1 Aug 2012 21:12:24 +0200
Subject: [PATCH 3/3] Fix index file's mtime, which can differ from TIMESTAMP.

This enables Portage to reliably query for remote indices with
HTTP-header If-Modified-Since.

Without this patch mtime is greater than TIMESTAMP for large
indices and slow storages - because writing a large file takes
time. If the difference spans a second (TIMESTAMP 08:00:00, mtime
08:00:01), then Portage will always fetch the remote index because
it will appear being modified (mtime is used there) after the copy
has been made (local copy's TIMESTAMP is used here).
---
 pym/portage/dbapi/bintree.py |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/pym/portage/dbapi/bintree.py b/pym/portage/dbapi/bintree.py
index 16ae8ec..0367503 100644
--- a/pym/portage/dbapi/bintree.py
+++ b/pym/portage/dbapi/bintree.py
@@ -1186,9 +1186,13 @@ class binarytree(object):
 			pkgindex.packages.append(d)
 
 			self._update_pkgindex_header(pkgindex.header)
-			f = atomic_ofstream(os.path.join(self.pkgdir, "Packages"))
+			pkgindex_filename = os.path.join(self.pkgdir, "Packages")
+			f = atomic_ofstream(pkgindex_filename)
 			pkgindex.write(f)
 			f.close()
+			# some seconds might have elapsed since TIMESTAMP
+			atime = mtime = long(pkgindex.header["TIMESTAMP"])
+			os.utime(pkgindex_filename, (atime, mtime))
 		finally:
 			if pkgindex_lock:
 				unlockfile(pkgindex_lock)
-- 
1.7.8.6

Re: [gentoo-portage-dev] [PATCH] portage: HTTP if-modified-since and compression

[Zac Medico]

On 08/01/2012 04:32 PM, Mark Kubacki wrote:
> Hi Portage devs,
> 
> The attached patches fix some issues I've noticed as maintainer and
> user of Gentoo binhost(s). They're made against master/HEAD and can
> easily be backported to 2.1*.
> 
> The first patch enables Portage to skip downloading a remote index if
> the local copy is recent enough. E.g., the remote index didn't change
> between to "emerge"-runs. This is done by setting "If-Modified-Since"
> request-header. The server responds with HTTP code 304 and Portage
> doesn't even load a single byte of the (large) index file.
> 
> By the second patch Portage will download remote indices—which are
> text-files after all—compressed, if the remote server supports that.
> Although de-compression introduces a small latency, this will save
> bandwidth and transmission time. If the index needs to be fetched at
> all, that is (see the patch above).
> 
> An index' TIMESTAMP entry is set before the corresponding file gets
> written. If the difference between TIMESTAMP and modification time
> ("mtime") is greater than or the times span one second, remote index
> files will be loaded despite the "If-Modified-Since" header. This is
> because TIMESTAMP of the local copy is compared with the remote index'
> "mtime". The third patch fixes that by setting "mtime" = TIMESTAMP.
> 

Thanks, I've applied your patches:

http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=e06cb6d66db37ac7ab77acf65038b1f770c13c96
http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=cbebf76d8e5666aad4984f87c2be83d474fe5a7e
http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=13abe0398fbe724218c8c9ac2597ebe15d7db7e1

I made a few trivial change in order to make them compatible with
python3. Also, I made it use the old behavior for protocols other than
http and https, in order to avoid issues with ftp like this one:

https://bugs.gentoo.org/show_bug.cgi?id=415579
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] portage: HTTP if-modified-since and compression

[Zac Medico]

On 08/01/2012 06:02 PM, Zac Medico wrote:
> On 08/01/2012 04:32 PM, Mark Kubacki wrote:
>> Hi Portage devs,
>>
>> The attached patches fix some issues I've noticed as maintainer and
>> user of Gentoo binhost(s). They're made against master/HEAD and can
>> easily be backported to 2.1*.
>>
>> The first patch enables Portage to skip downloading a remote index if
>> the local copy is recent enough. E.g., the remote index didn't change
>> between to "emerge"-runs. This is done by setting "If-Modified-Since"
>> request-header. The server responds with HTTP code 304 and Portage
>> doesn't even load a single byte of the (large) index file.
>>
>> By the second patch Portage will download remote indices—which are
>> text-files after all—compressed, if the remote server supports that.
>> Although de-compression introduces a small latency, this will save
>> bandwidth and transmission time. If the index needs to be fetched at
>> all, that is (see the patch above).
>>
>> An index' TIMESTAMP entry is set before the corresponding file gets
>> written. If the difference between TIMESTAMP and modification time
>> ("mtime") is greater than or the times span one second, remote index
>> files will be loaded despite the "If-Modified-Since" header. This is
>> because TIMESTAMP of the local copy is compared with the remote index'
>> "mtime". The third patch fixes that by setting "mtime" = TIMESTAMP.
>>
> 
> Thanks, I've applied your patches:
> 
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=e06cb6d66db37ac7ab77acf65038b1f770c13c96
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=cbebf76d8e5666aad4984f87c2be83d474fe5a7e
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=13abe0398fbe724218c8c9ac2597ebe15d7db7e1
> 
> I made a few trivial change in order to make them compatible with
> python3. Also, I made it use the old behavior for protocols other than
> http and https, in order to avoid issues with ftp like this one:
> 
> https://bugs.gentoo.org/show_bug.cgi?id=415579
> 

Fix python2 http password breakage:

http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=fbeb8101b20e232b2e8c55c9554b5fc9c5c72089

BUG: The if_modified_since parameter appears to be ignored when using
http password authentication.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] portage: HTTP if-modified-since and compression

[Mark Kubacki]

2012/8/2 Zac Medico <zmedico@gentoo.org>:
> On 08/01/2012 06:02 PM, Zac Medico wrote:
>>
>> Thanks, I've applied your patches:
>>
>> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=e06cb6d66db37ac7ab77acf65038b1f770c13c96
>> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=cbebf76d8e5666aad4984f87c2be83d474fe5a7e
>> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=13abe0398fbe724218c8c9ac2597ebe15d7db7e1
>>
>> I made a few trivial change in order to make them compatible with
>> python3. Also, I made it use the old behavior for protocols other than
>> http and https, in order to avoid issues with ftp like this one:
>>
>> https://bugs.gentoo.org/show_bug.cgi?id=415579
>>
>
> Fix python2 http password breakage:
>
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=fbeb8101b20e232b2e8c55c9554b5fc9c5c72089
>
> BUG: The if_modified_since parameter appears to be ignored when using
> http password authentication.

Hi Zac,

In one word: Great! I love your modifications. Thank you!

Regarding functionality – there is still some room for more
optimizations and more features. For example, if the local copy is no
older than x seconds then there's no need to contact any remote
server. Expect patches.

As for the bug. As long as the "If-Modified-Since" header is sent
Portage has done its job. Some servers use the header as "ETag"
replacement and don't do the more costly greater-than comparison (see
also [1]; TIMESTAMP_TOLERANCE should be a configuration option so
users can set it to 0 now that the "mtime"-patch has been accepted).
And, BaseHandler are chained automatically by "build_opener".
Nevertheless, I will look into the whole issue the next days.

-- 
Grüße, Mark

Re: [gentoo-portage-dev] [PATCH] portage: HTTP if-modified-since and compression

[Zac Medico]

On 08/02/2012 12:57 PM, Mark Kubacki wrote:
> As for the bug. As long as the "If-Modified-Since" header is sent
> Portage has done its job.

Maybe I just observed a quirk of thttpd. The if_modified_since parameter
appeared to work as long as I didn't use password authentication.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] portage: HTTP if-modified-since and compression

[Brian Dolbec]

[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]

On Thu, 2012-08-02 at 21:57 +0200, Mark Kubacki wrote:
> Hi Zac,
> 
> In one word: Great! I love your modifications. Thank you!
> 
> Regarding functionality – there is still some room for more
> optimizations and more features. For example, if the local copy is no
> older than x seconds then there's no need to contact any remote
> server. Expect patches.
> 
> As for the bug. As long as the "If-Modified-Since" header is sent
> Portage has done its job. Some servers use the header as "ETag"
> replacement and don't do the more costly greater-than comparison (see
> also [1]; TIMESTAMP_TOLERANCE should be a configuration option so
> users can set it to 0 now that the "mtime"-patch has been accepted).
> And, BaseHandler are chained automatically by "build_opener".
> Nevertheless, I will look into the whole issue the next days.
> 

Mark, I did similar for the layman-2.0 code which has been running with
the header info for quite a while now.  After it had been running for a
good amount of time I put in a request to infra for some usage stats.

The If-Modified-Since header does make a big difference for layman.
Now I just really need to make a good blog post with a few graphs of the
data.
You can view the results on this bug if your interested:
https://bugs.gentoo.org/show_bug.cgi?id=398465

-- 
Brian Dolbec <dolsen@gentoo.org>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [gentoo-portage-dev] [PATCH] portage: HTTP if-modified-since and compression

[W-Mark Kubacki]

On Thu, Aug 02, 2012 at 06:29:31PM -0700, Brian Dolbec wrote:
> On Thu, 2012-08-02 at 21:57 +0200, Mark Kubacki wrote:
> > 
> > Regarding functionality – there is still some room for more
> > optimizations and more features. For example, if the local copy is no
> > older than x seconds then there's no need to contact any remote
> > server. Expect patches.
> > 
> 
> Mark, I did similar for the layman-2.0 code which has been running with
> the header info for quite a while now.  After it had been running for a
> good amount of time I put in a request to infra for some usage stats.
> 
> The If-Modified-Since header does make a big difference for layman.
> Now I just really need to make a good blog post with a few graphs of the
> data.
> You can view the results on this bug if your interested:
> https://bugs.gentoo.org/show_bug.cgi?id=398465
> 

Brian, thanks for the stats and the pointer to layman. I guess we both 
see the opportunity to share some experiences and code. Layman can 
benefit from adding compression and I need to integrate your notices 
about Py2/Py3 compatibility.

If it is okay with Zac I will refactor and improve the URL-fetching some 
more. Following redirects, a proper auth-handler and 'identificator' 
comes to mind. You could copy the final handlers, then.

Portage's 'emerge' currently contacts remote hosts whenever it is run 
and this adds a noticeable delay. In the best case even the 304 (not 
modified) responses are avoid wherever possible. So in the end success 
of Portage's caching will not be measurable by a 200-to-304 ratio.

-- 
Mark

[1] http://trac.nginx.org/nginx/ticket/93 – discussion about unintuitive 
but valid handling of the If-Modified-Since header

Re: [gentoo-portage-dev] [PATCH] portage: HTTP if-modified-since and compression

[Brian Dolbec]

[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]

On Fri, 2012-08-03 at 11:33 +0200, W-Mark Kubacki wrote:
> Brian, thanks for the stats and the pointer to layman. I guess we both 
> see the opportunity to share some experiences and code. Layman can 
> benefit from adding compression and I need to integrate your notices 
> about Py2/Py3 compatibility.
> 

I haven't quite gotten all that is needed for py2/py3 compatibility
done.  Also I could not get it to work with both py2 and py3 with the
same code.  I have a little more to modify for a clean 2to3 run on the
code to work without additional patching.

> If it is okay with Zac I will refactor and improve the URL-fetching some 
> more. Following redirects, a proper auth-handler and 'identificator' 
> comes to mind. You could copy the final handlers, then.
> 

:)  my laziness thanks you in advance 

> Portage's 'emerge' currently contacts remote hosts whenever it is run 
> and this adds a noticeable delay. In the best case even the 304 (not 
> modified) responses are avoid wherever possible. So in the end success 
> of Portage's caching will not be measurable by a 200-to-304 ratio.
> 

Layman does similar, but not every run, just -L, -f, -S, -s options,
which is why I added the headers...

hmm, adding the if >=x time fetch skip is a good idea.

-- 
Brian Dolbec <dolsen@gentoo.org>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]