want a way to mark transport operations as cacheable

Buggy proxies (returning stale data) have been encountered in the past.
If a cache gives us older versions of some files, and newer versions of others, we'll see the repository as broken.
Therefore, bzr explicitly bypass cache by using a 'no-cache' directive as Roland rightly deduced.

The attached patch disables that behavior. Use at your own risk.

There are no reliable ways to detect whether or not a proxy will behave correctly regarding caching so bzr will continue to use 'no-cache' directives.

But we intend to make it more network-friendly by other means.

On Sun, 2007-11-25 at 13:58 +0000, vila wrote:
> There are no reliable ways to detect whether or not a proxy will behave
> correctly regarding caching so bzr will continue to use 'no-cache'
> directives.
>
> But we intend to make it more network-friendly by other means.

I think we should keep this open a bit longer.

Specifically, the current code assumes all files are equally hard to
cache. This is bogus.

e.g.
foo.knit: not safe to cache
foo.kndx: not safe to cache
branch.conf: not safe to cache
3847868641027641243.pack: safe to cache
3847868641027641243.iix: safe to cache
branch-format: safe to cache
format: safe to cache
lock/*: not safe to cache.

etc.

-Rob
--
GPG key available at: <http://www.robertcollins.net/keys.txt>.

Right, bug reopened then.

But the transport layer will not identify such files by itself.

Right, so this should be a per-file option, defaulting to 'don't
cache'. It could be done as an option on each read, as an option on
the transport, or by introducing a new method to open a file for
reading and making this part of the state of that object.

As an aside, is squid smart enough to cache readv() requests? Does it just request the whole file, and then return the pieces?

Otherwise, I'm guessing that Bazaar is still going to be unfriendly to caches, because it makes requests for pieces of the files. (which is sometimes the whole file, but the proxy wouldn't know whether 0-1111 is the whole file, or if there is a byte 1112.)

On Mon, 2007-11-26 at 20:41 +0000, John A Meinel wrote:
> As an aside, is squid smart enough to cache readv() requests? Does it
> just request the whole file, and then return the pieces?
>
> Otherwise, I'm guessing that Bazaar is still going to be unfriendly to
> caches, because it makes requests for pieces of the files. (which is
> sometimes the whole file, but the proxy wouldn't know whether 0-1111 is
> the whole file, or if there is a byte 1112.)

A sufficiently complex range will cause squid to read the whole file -
and serve later responses from cache (unless cache busting headers are
used).

If cache busting headers are used by the client it does not attempt to
process the ranges at all and just forwards them upstream.

-Rob
--
GPG key available at: <http://www.robertcollins.net/keys.txt>.

Robert Collins wrote:
> I think we should keep this open a bit longer.
>
> Specifically, the current code assumes all files are equally hard to
> cache. This is bogus.
>
> e.g.
> foo.knit: not safe to cache
> foo.kndx: not safe to cache
> branch.conf: not safe to cache
> 3847868641027641243.pack: safe to cache
> 3847868641027641243.iix: safe to cache
> branch-format: safe to cache
> format: safe to cache
> lock/*: not safe to cache.
>
> etc.

branch-format and format aren't really safe to cache. What if someone
runs 'bzr upgrade'?
--

On Wed, 2007-11-28 at 00:56 +0000, Matt Nordhoff wrote:
> Robert Collins wrote:
> > I think we should keep this open a bit longer.
> >
> > Specifically, the current code assumes all files are equally hard to
> > cache. This is bogus.
> >
> > e.g.
> > foo.knit: not safe to cache
> > foo.kndx: not safe to cache
> > branch.conf: not safe to cache
> > 3847868641027641243.pack: safe to cache
> > 3847868641027641243.iix: safe to cache
> > branch-format: safe to cache
> > format: safe to cache
> > lock/*: not safe to cache.
> >
> > etc.
>
> branch-format and format aren't really safe to cache. What if someone
> runs 'bzr upgrade'?

Well, there's safe and safe I guess.

on the one hand those files change very rarely.

on the other they matter quite a bit :).

-Rob
--
GPG key available at: <http://www.robertcollins.net/keys.txt>.

lifeless said on IRC:

http://www.squid-cache.org/Versions/v2/2.6/changesets/11996.patch
 you need http://www.squid-cache.org/Versions/v2/2.6/changesets/SQUID_2_6_STABLE19.html
I wonder if detecting squid versions with the bug would be useful, give a warning

Debian says this was fixed by the introduction of the packs format, which is a bit weird. Oh well.

It looks like the remaining part of this bug is that we want a way to say, on a particular Transport operation, that the file we're reading is potentially cacheable. This would be safe to set if the file is never modified once it has been written, which is true for pack and index contents.

I'm going to mark vincent's old patch as "not a patch" as it's not actually a foundation for closing this bug.

I propose the following:
 - we add a flag 'permit_cached' to transport.get, get_bytes, readv.
 - that flag gets passed down the stack to the header functions vila changes in his draft patch.

On 3 April 2010 08:28, Robert Collins <email address hidden> wrote:
> I propose the following:
>  - we add a flag 'permit_cached' to transport.get, get_bytes, readv.

I wonder if this ought to be richer than a boolean: must-revalidate vs
disable caches altogether, and perhaps the concept of
if-modified-since (though bzr itself may not be able to use that yet.)

--
Martin <http://launchpad.net/~mbp/>

We could make it richer yes; do we need that though?

On 6 April 2010 15:25, Robert Collins <email address hidden> wrote:
> We could make it richer yes; do we need that though?

I don't know but it seems possible. Some files are just about
guaranteed to be immutable once written, like pack files. Some are
modified but then otherwise cacheable, and in that case
must-revalidate would make sense.

At any rate I wouldn't want a proliferation of options on every single call.

Perhaps it should be per-transport state and then you can create
different transport objects. Or a dict of options per call.

--
Martin <http://launchpad.net/~mbp/>

Woaw ! Thanks to Vincent Ladeuil (vila) wrote on 2007-11-21: Re: bzr shouldn't bypass http caches

Before:

time bzr branch http://url -Dbytes
Branched 8 revisions.
Transferred: 6470kB (185.6kB/s r:6444kB w:26kB)

real 0m35.063s
user 0m0.688s
sys 0m0.168s

After:

time bzr branch http://url -Dbytes
Branched 8 revisions.
Transferred: 6469kB (431.2kB/s r:6445kB w:24kB)

real 0m15.226s
user 0m0.620s
sys 0m0.128s

bzr --version
Bazaar (bzr) 2.5.0
  Python interpreter: /usr/bin/python 2.6.5
  Python standard library: /usr/lib/python2.6
  Platform: Linux-3.0.0-17-generic-x86_64-with-Ubuntu-10.04-lucid
  bzrlib: /usr/lib/pymodules/python2.6/bzrlib
  Bazaar configuration: /home/utilisateurs/eric.seigne/.bazaar
  Bazaar log file: /home/utilisateurs/eric.seigne/.bzr.log