Bug #402669 “2a fetch does not use 'skinny' groups” : Bugs : Bazaar

Revision history for this message

Robert Collins (lifeless) wrote on 2009-07-21: Re: [Bug 402669] [NEW] 2a fetch does not use 'skinny' groups

#1

On Tue, 2009-07-21 at 18:53 +0000, John A Meinel wrote:
>
>
> 1) Generally when fetching we are asking for a set of keys, and we
> don't really
> make assumptions about keys that we aren't requesting.
>
> For example, we say "give me keys A, B, C" but we don't also say "and
> not D".
>
> We might be able to infer some of that if we have a graph for the
> given keys
> (we have a graph for text keys, but no graph for chk pages.)

We already have a solution for this - the streaming protocol permits a
'and I need X text' response to a stream; so rather than worrying about
what the server will have, just let the server tell us.

> 2) This also might have bad results interacting with stacking.

> ...

> However, it cannot expand the stream
> as it comes across the wire, and would need to buffer.

Same answer as for 1 - only send the bytes we know the new revs
introduced, and if a parent is missing the recipient will tell us.

> 3) There are probably other edge cases that could be triggered with
> combinations of stacking/ghosts/and smart fetches (since a stacked
> repository
> doesn't have access to the backing repository during smart fetch.)

I'm reasonably sure the current answer is sufficient.

> 4) We might be able to make a bit more use out of things like
> SearchResult,
> which has a bit more knowledge about things that are accessible and
> things that
> are not.
>
> We could change the get_record_stream() api to be a bit more concrete
> about things that could be used as omitted compression parents,
> allowing the
> logic of what should be grouped up to be computed at a higher level,
> and then
> passed down.

I don't think thats really needed, though I do want the stream interface
to be as powerful as needed.

> 5) This would cause some amount of server overhead, as it has to
> generate new
> compressed forms. This may be more overhead than similar systems,
> since none of
> the groupcompress delta 'recipies' are valid outside of the specific
> group they
> are in. (Versus chained deltas, where if you insert a new delta at the
> front or
> back of the chain, the intermediate deltas are all still valid.)

I think this is tolerable. We're already decompressing everything we
receive, and other systems do recombine texts from streams.

-Rob

On Tue, 2009-07-21 at 18:53 +0000, John A Meinel wrote:
> 
> 
> 1) Generally when fetching we are asking for a set of keys, and we
> don't really
> make assumptions about keys that we aren't requesting.
> 
> For example, we say "give me keys A, B, C" but we don't also say "and
> not D".
> 
> We might be able to infer some of that if we have a graph for the
> given keys
> (we have a graph for text keys, but no graph for chk pages.)

We already have a solution for this - the streaming protocol permits a
'and I need X text' response to a stream; so rather than worrying about
what the server will have, just let the server tell us.

> 2) This also might have bad results interacting with stacking.

> ...

>  However, it cannot expand the stream
> as it comes across the wire, and would need to buffer.

Same answer as for 1 - only send the bytes we know the new revs
introduced, and if a parent is missing the recipient will tell us.

> 3) There are probably other edge cases that could be triggered with
> combinations of stacking/ghosts/and smart fetches (since a stacked
> repository
> doesn't have access to the backing repository during smart fetch.)

I'm reasonably sure the current answer is sufficient.

> 4) We might be able to make a bit more use out of things like
> SearchResult,
> which has a bit more knowledge about things that are accessible and
> things that
> are not.
> 
> We could change the get_record_stream() api to be a bit more concrete
> about things that could be used as omitted compression parents,
> allowing the
> logic of what should be grouped up to be computed at a higher level,
> and then
> passed down.

I don't think thats really needed, though I do want the stream interface
to be as powerful as needed.

> 5) This would cause some amount of server overhead, as it has to
> generate new
> compressed forms. This may be more overhead than similar systems,
> since none of
> the groupcompress delta 'recipies' are valid outside of the specific
> group they
> are in. (Versus chained deltas, where if you insert a new delta at the
> front or
> back of the chain, the intermediate deltas are all still valid.)

I think this is tolerable. We're already decompressing everything we
receive, and other systems do recombine texts from streams.

-Rob

Revision history for this message

Martin Pool (mbp) wrote on 2009-07-22:

#2

This seems like a dupe of bug 391902? (Maybe mark that as a dupe of
this since there's more text here.)

Revision history for this message

John A Meinel (jameinel) wrote on 2009-07-22:

#3

Download full text (3.6 KiB)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> On Tue, 2009-07-21 at 18:53 +0000, John A Meinel wrote:
>>
>> 1) Generally when fetching we are asking for a set of keys, and we
>> don't really
>> make assumptions about keys that we aren't requesting.
>>
>> For example, we say "give me keys A, B, C" but we don't also say "and
>> not D".
>>
>> We might be able to infer some of that if we have a graph for the
>> given keys
>> (we have a graph for text keys, but no graph for chk pages.)
>
> We already have a solution for this - the streaming protocol permits a
> 'and I need X text' response to a stream; so rather than worrying about
> what the server will have, just let the server tell us.

So we can do this, but it also means that a specific *group* will need
to be buffered somewhere while we determine that we don't have the
necessary text, and come back later to fill it in.

We can't even write the group into the current .pack file, because until
we fill it in, all the data in it is invalid.

So yes we can, but it isn't a trivial extension of what we do today.

>
>> 2) This also might have bad results interacting with stacking.
>
>> ...
>
>> However, it cannot expand the stream
>> as it comes across the wire, and would need to buffer.
>
> Same answer as for 1 - only send the bytes we know the new revs
> introduced, and if a parent is missing the recipient will tell us.
>

So it makes 1 critical new assumption that we haven't made before. Which
is that we also will now send references to bytes that we think were
referenced by parents of the new revs. I suppose we started doing
something similar for inventories, and we suffered a fairly large
fallout from that.

I suppose if we wanted to avoid per-file graphs we could just grab the
parent inventories and find the set of all referenced keys (which we
half-do today), and then use that as a set of possible texts to compress
against. It is a bit tricky, because we need to figure out which ones
would make *good* compression texts, though file-ids help us here.

...

>
>> 5) This would cause some amount of server overhead, as it has to
>> generate new
>> compressed forms. This may be more overhead than similar systems,
>> since none of
>> the groupcompress delta 'recipies' are valid outside of the specific
>> group they
>> are in. (Versus chained deltas, where if you insert a new delta at the
>> front or
>> back of the chain, the intermediate deltas are all still valid.)
>
> I think this is tolerable. We're already decompressing everything we
> receive, and other systems do recombine texts from streams.
>
> -Rob
>

Not actually true anymore. Though from my timings the time to extract
the texts is significantly faster than the time to compute the sha1 sum
for that amount of data, so we *could* do it.

The bigger issue here is that it changes quite a bit about how
get_record_stream works. As now we will

1) Sometimes transmit the blocks as-is
2) Sometimes want to rebuild a group with keys that will include and
then omit parent keys
3) Sometimes just take a block and strip out the unwanted bits

It doesn't really help that groups aren't self-describing anymore (you
have ...

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> On Tue, 2009-07-21 at 18:53 +0000, John A Meinel wrote:
>>
>> 1) Generally when fetching we are asking for a set of keys, and we
>> don't really
>> make assumptions about keys that we aren't requesting.
>>
>> For example, we say "give me keys A, B, C" but we don't also say "and
>> not D".
>>
>> We might be able to infer some of that if we have a graph for the
>> given keys
>> (we have a graph for text keys, but no graph for chk pages.)
> 
> We already have a solution for this - the streaming protocol permits a
> 'and I need X text' response to a stream; so rather than worrying about
> what the server will have, just let the server tell us.

So we can do this, but it also means that a specific *group* will need
to be buffered somewhere while we determine that we don't have the
necessary text, and come back later to fill it in.

We can't even write the group into the current .pack file, because until
we fill it in, all the data in it is invalid.

So yes we can, but it isn't a trivial extension of what we do today.

> 
>> 2) This also might have bad results interacting with stacking.
> 
>> ...
> 
>>  However, it cannot expand the stream
>> as it comes across the wire, and would need to buffer.
> 
> Same answer as for 1 - only send the bytes we know the new revs
> introduced, and if a parent is missing the recipient will tell us.
>

So it makes 1 critical new assumption that we haven't made before. Which
is that we also will now send references to bytes that we think were
referenced by parents of the new revs. I suppose we started doing
something similar for inventories, and we suffered a fairly large
fallout from that.

I suppose if we wanted to avoid per-file graphs we could just grab the
parent inventories and find the set of all referenced keys (which we
half-do today), and then use that as a set of possible texts to compress
against. It is a bit tricky, because we need to figure out which ones
would make *good* compression texts, though file-ids help us here.

...

> 
>> 5) This would cause some amount of server overhead, as it has to
>> generate new
>> compressed forms. This may be more overhead than similar systems,
>> since none of
>> the groupcompress delta 'recipies' are valid outside of the specific
>> group they
>> are in. (Versus chained deltas, where if you insert a new delta at the
>> front or
>> back of the chain, the intermediate deltas are all still valid.)
> 
> I think this is tolerable. We're already decompressing everything we
> receive, and other systems do recombine texts from streams.
> 
> -Rob
>

Not actually true anymore. Though from my timings the time to extract
the texts is significantly faster than the time to compute the sha1 sum
for that amount of data, so we *could* do it.

The bigger issue here is that it changes quite a bit about how
get_record_stream works. As now we will

1) Sometimes transmit the blocks as-is
2) Sometimes want to rebuild a group with keys that will include and
then omit parent keys
3) Sometimes just take a block and strip out the unwanted bits

It doesn't really help that groups aren't self-describing anymore (you
have to read the index to know exactly what is in a group). Though I'm
still pretty sure that the tradeoff is worthwhile (20-40% overhead for
making it self describing was a bit much.)

Anyway, it is certainly something that should be worked on. Just not
something trivial.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkpnUpkACgkQJdeBCYSNAAOrXQCgrgkwygLG51tCBeIDRNU4pg1b
WAwAnirW0bgEf4rJnwkdJ/GSx4+Q1tiU
=pHuS
-----END PGP SIGNATURE-----

Revision history for this message

Robert Collins (lifeless) wrote on 2009-07-22:

#4

On Wed, 2009-07-22 at 17:55 +0000, John A Meinel wrote:
>
> So we can do this, but it also means that a specific *group* will need
> to be buffered somewhere while we determine that we don't have the
> necessary text, and come back later to fill it in.
>
> We can't even write the group into the current .pack file, because
> until
> we fill it in, all the data in it is invalid.

At the cost of reprocessing we could do it this way:
- write the pack as-received), until we link it in noone else is
permitted to use it anyway.
- check it and report missing
- when we get a second stream, write that, and then check both
- if nothing else is missing, combine the two to a final pack that is
complete

I expect that we may need to permit a third stream, because until we can
decompress all the chk pages we can't tell if we have missing inv data..
or perhaps we can. Anyhow, thats not so different to what we do in 1.9
format streaming situations.

> So it makes 1 critical new assumption that we haven't made before.
> Which
> is that we also will now send references to bytes that we think were
> referenced by parents of the new revs. I suppose we started doing
> something similar for inventories, and we suffered a fairly large
> fallout from that.

the inventory one was to do with logical deltas, not references as such;
the protocol Andrew and I wrote is actually generic at this level and
doesn't need changing to meet skinny groups/arbitrary compression
references. The only thing that may/will need changing is that the
recipient should discard revisions that are sent as compression fillers,
rather than keeping them (to preserve the
revisions-imply-revision-content assertion).

> I suppose if we wanted to avoid per-file graphs we could just grab the
> parent inventories and find the set of all referenced keys (which we
> half-do today), and then use that as a set of possible texts to
> compress
> against. It is a bit tricky, because we need to figure out which ones
> would make *good* compression texts, though file-ids help us here.

We could, but the cheapest thing in terms of processing is to just
assume the recipient has everything :).
..
> > I think this is tolerable. We're already decompressing everything we
> > receive, and other systems do recombine texts from streams.
> >
> > -Rob
> >
>
> Not actually true anymore. Though from my timings the time to extract
> the texts is significantly faster than the time to compute the sha1
> sum
> for that amount of data, so we *could* do it.

Ah, so we calculate the sha-if-we-were-to-decompress? Surely we are
decompressing the CHK pages though to check for missing references?
Thats a correctness bug if we're not.

-Rob

On Wed, 2009-07-22 at 17:55 +0000, John A Meinel wrote:
> 
> So we can do this, but it also means that a specific *group* will need
> to be buffered somewhere while we determine that we don't have the
> necessary text, and come back later to fill it in.
> 
> We can't even write the group into the current .pack file, because
> until
> we fill it in, all the data in it is invalid.

At the cost of reprocessing we could do it this way:
- write the pack as-received), until we link it in noone else is
permitted to use it anyway.
- check it and report missing
- when we get a second stream, write that, and then check both
- if nothing else is missing, combine the two to a final pack that is
complete

I expect that we may need to permit a third stream, because until we can
decompress all the chk pages we can't tell if we have missing inv data..
or perhaps we can. Anyhow, thats not so different to what we do in 1.9
format streaming situations.

> So it makes 1 critical new assumption that we haven't made before.
> Which
> is that we also will now send references to bytes that we think were
> referenced by parents of the new revs. I suppose we started doing
> something similar for inventories, and we suffered a fairly large
> fallout from that.

the inventory one was to do with logical deltas, not references as such;
the protocol Andrew and I wrote is actually generic at this level and
doesn't need changing to meet skinny groups/arbitrary compression
references. The only thing that may/will need changing is that the
recipient should discard revisions that are sent as compression fillers,
rather than keeping them (to preserve the
revisions-imply-revision-content assertion).

> I suppose if we wanted to avoid per-file graphs we could just grab the
> parent inventories and find the set of all referenced keys (which we
> half-do today), and then use that as a set of possible texts to
> compress
> against. It is a bit tricky, because we need to figure out which ones
> would make *good* compression texts, though file-ids help us here.

We could, but the cheapest thing in terms of processing is to just
assume the recipient has everything :).
..
> > I think this is tolerable. We're already decompressing everything we
> > receive, and other systems do recombine texts from streams.
> > 
> > -Rob
> > 
> 
> Not actually true anymore. Though from my timings the time to extract
> the texts is significantly faster than the time to compute the sha1
> sum
> for that amount of data, so we *could* do it.

Ah, so we calculate the sha-if-we-were-to-decompress? Surely we are
decompressing the CHK pages though to check for missing references?
Thats a correctness bug if we're not.

-Rob

Revision history for this message

John A Meinel (jameinel) wrote on 2009-07-22:

#5

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...

>>> I think this is tolerable. We're already decompressing everything we
>>> receive, and other systems do recombine texts from streams.
>>>
>>> -Rob
>>>
>> Not actually true anymore. Though from my timings the time to extract
>> the texts is significantly faster than the time to compute the sha1
>> sum
>> for that amount of data, so we *could* do it.
>
> Ah, so we calculate the sha-if-we-were-to-decompress? Surely we are
> decompressing the CHK pages though to check for missing references?
> Thats a correctness bug if we're not.
>
> -Rob
>

So we decompress the chk pages as we read, in order to find references.
Absolutely true. We do the same for 'inventories'.

We don't decompress the Revisions, Texts or Signatures.

In the past, we did some amount of validation at extraction time for all
knit texts. However, when you looked into it, we only validated the sha1
sum against the value that was stored *in the knit record*. Which
basically only validates that he knit-delta is correct.

Since Groupcompress can't (currently) have invalid datas, there wasn't
any point to wasting space recording "this is the sha1sum of the
following 10 bytes".

What we *should* do, is to change the streaming code so that when it
finds the chk page keys it then validates the referenced bytes that they
match their chk key. And then when it finds file keys it should also
track the sha1 of the associated reference, and then when we insert the
text stream we should validate the text bytes match the sha1 sum that we
saw in the inventory.

The reason we *don't* do that, is because the information isn't
available at the various times.

The *Source* parses all of the pages to find the referenced keys, the
*Sink* does not. (Arguably it should, as long as we have a way to avoid
double-handling when the Source and Sink are both in-process.)

Further, the stream for inventory is conceptually separate from the
stream for texts. One possibility would be something like:

invs = source.inventories.get_record_stream(revision_keys)
chk_keys = target.inventories.insert_stream(invs)
chk_stream = source.chk_bytes.get_record_stream(chk_keys)
file_keys, file_key_sha1s = target.chk_bytes.insert_stream(chk_stream,
validate_from_keys=True)
text_stream = source.texts.get_record_stream(file_keys)
target.texts.insert_stream(text_stream, file_key_sha1s)

I'm hand waving, the issue is that the extra state you extract from one
stream needs to be passed to the insert_stream() side of the equation.
Which is a fairly significant API break, so it should be done with some
care. (to avoid re-doing it lots of times.)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkpnpkUACgkQJdeBCYSNAAMYZwCgtkkyjQxaFRltTeL46fr6TQuD
31AAoItG6NQeAM/Pp7Txsd64IJvrimVz
=cNaz
-----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...

>>> I think this is tolerable. We're already decompressing everything we
>>> receive, and other systems do recombine texts from streams.
>>>
>>> -Rob
>>>
>> Not actually true anymore. Though from my timings the time to extract
>> the texts is significantly faster than the time to compute the sha1
>> sum
>> for that amount of data, so we *could* do it.
> 
> Ah, so we calculate the sha-if-we-were-to-decompress? Surely we are
> decompressing the CHK pages though to check for missing references?
> Thats a correctness bug if we're not.
> 
> -Rob
>

So we decompress the chk pages as we read, in order to find references.
Absolutely true. We do the same for 'inventories'.

We don't decompress the Revisions, Texts or Signatures.

In the past, we did some amount of validation at extraction time for all
knit texts. However, when you looked into it, we only validated the sha1
sum against the value that was stored *in the knit record*. Which
basically only validates that he knit-delta is correct.

Since Groupcompress can't (currently) have invalid datas, there wasn't
any point to wasting space recording "this is the sha1sum of the
following 10 bytes".

What we *should* do, is to change the streaming code so that when it
finds the chk page keys it then validates the referenced bytes that they
match their chk key. And then when it finds file keys it should also
track the sha1 of the associated reference, and then when we insert the
text stream we should validate the text bytes match the sha1 sum that we
saw in the inventory.

The reason we *don't* do that, is because the information isn't
available at the various times.

The *Source* parses all of the pages to find the referenced keys, the
*Sink* does not. (Arguably it should, as long as we have a way to avoid
double-handling when the Source and Sink are both in-process.)

Further, the stream for inventory is conceptually separate from the
stream for texts. One possibility would be something like:

invs = source.inventories.get_record_stream(revision_keys)
chk_keys = target.inventories.insert_stream(invs)
chk_stream = source.chk_bytes.get_record_stream(chk_keys)
file_keys, file_key_sha1s = target.chk_bytes.insert_stream(chk_stream,
 validate_from_keys=True)
text_stream = source.texts.get_record_stream(file_keys)
target.texts.insert_stream(text_stream, file_key_sha1s)

I'm hand waving, the issue is that the extra state you extract from one
stream needs to be passed to the insert_stream() side of the equation.
Which is a fairly significant API break, so it should be done with some
care. (to avoid re-doing it lots of times.)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkpnpkUACgkQJdeBCYSNAAMYZwCgtkkyjQxaFRltTeL46fr6TQuD
31AAoItG6NQeAM/Pp7Txsd64IJvrimVz
=cNaz
-----END PGP SIGNATURE-----

Revision history for this message

Robert Collins (lifeless) wrote on 2009-07-23:

#6

On Wed, 2009-07-22 at 23:52 +0000, John A Meinel wrote:

> So we decompress the chk pages as we read, in order to find references.
> Absolutely true. We do the same for 'inventories'.
>
> We don't decompress the Revisions, Texts or Signatures.

Arguably we should be decompressing the revisions to check the inventory
validator.

> Since Groupcompress can't (currently) have invalid datas, there wasn't
> any point to wasting space recording "this is the sha1sum of the
> following 10 bytes".

Erm, that doesn't really follow. Its trivial to change a group so that
it still parses correctly but generates different sha1's. That said, a
good reason not to have the sha1sum is that the sha1sum is trivially
editable at the same time.

> What we *should* do, is to change the streaming code so that when it
> finds the chk page keys it then validates the referenced bytes that they
> match their chk key.

I *think* this is already in place for extract, if not we should open a
bug on it.

> And then when it finds file keys it should also
> track the sha1 of the associated reference, and then when we insert the
> text stream we should validate the text bytes match the sha1 sum that we
> saw in the inventory.

Yes.

> The reason we *don't* do that, is because the information isn't
> available at the various times.

We can do it lazily - track the sha1s we see, and when we see references
accumulate; then compare at the end.

> The *Source* parses all of the pages to find the referenced keys, the
> *Sink* does not. (Arguably it should, as long as we have a way to avoid
> double-handling when the Source and Sink are both in-process.)

Yup.

> Further, the stream for inventory is conceptually separate from the
> stream for texts. One possibility would be something like:
>
> invs = source.inventories.get_record_stream(revision_keys)
> chk_keys = target.inventories.insert_stream(invs)
> chk_stream = source.chk_bytes.get_record_stream(chk_keys)
> file_keys, file_key_sha1s = target.chk_bytes.insert_stream(chk_stream,
> validate_from_keys=True)
> text_stream = source.texts.get_record_stream(file_keys)
> target.texts.insert_stream(text_stream, file_key_sha1s)
>
> I'm hand waving, the issue is that the extra state you extract from one
> stream needs to be passed to the insert_stream() side of the equation.
> Which is a fairly significant API break, so it should be done with some
> care. (to avoid re-doing it lots of times.)

I disagree here; we should preserve a strict pipelining model, which is
what spiv and I have been working on. That permits e.g. forking a
process locally and multi tasking, working with http post, etc.

-Rob

On Wed, 2009-07-22 at 23:52 +0000, John A Meinel wrote:

> So we decompress the chk pages as we read, in order to find references.
> Absolutely true. We do the same for 'inventories'.
> 
> We don't decompress the Revisions, Texts or Signatures.

Arguably we should be decompressing the revisions to check the inventory
validator.

> Since Groupcompress can't (currently) have invalid datas, there wasn't
> any point to wasting space recording "this is the sha1sum of the
> following 10 bytes".

Erm, that doesn't really follow. Its trivial to change a group so that
it still parses correctly but generates different sha1's. That said, a
good reason not to have the sha1sum is that the sha1sum is trivially
editable at the same time.

> What we *should* do, is to change the streaming code so that when it
> finds the chk page keys it then validates the referenced bytes that they
> match their chk key.

I *think* this is already in place for extract, if not we should open a
bug on it.

>  And then when it finds file keys it should also
> track the sha1 of the associated reference, and then when we insert the
> text stream we should validate the text bytes match the sha1 sum that we
> saw in the inventory.

Yes.

> The reason we *don't* do that, is because the information isn't
> available at the various times.

We can do it lazily - track the sha1s we see, and when we see references
accumulate; then compare at the end.

> The *Source* parses all of the pages to find the referenced keys, the
> *Sink* does not. (Arguably it should, as long as we have a way to avoid
> double-handling when the Source and Sink are both in-process.)

Yup.

> Further, the stream for inventory is conceptually separate from the
> stream for texts. One possibility would be something like:
> 
> invs = source.inventories.get_record_stream(revision_keys)
> chk_keys = target.inventories.insert_stream(invs)
> chk_stream = source.chk_bytes.get_record_stream(chk_keys)
> file_keys, file_key_sha1s = target.chk_bytes.insert_stream(chk_stream,
>  validate_from_keys=True)
> text_stream = source.texts.get_record_stream(file_keys)
> target.texts.insert_stream(text_stream, file_key_sha1s)
> 
> I'm hand waving, the issue is that the extra state you extract from one
> stream needs to be passed to the insert_stream() side of the equation.
> Which is a fairly significant API break, so it should be done with some
> care. (to avoid re-doing it lots of times.)

I disagree here; we should preserve a strict pipelining model, which is
what spiv and I have been working on. That permits e.g. forking a
process locally and multi tasking, working with http post, etc.

-Rob

Revision history for this message

John A Meinel (jameinel) wrote on 2009-07-23:

#7

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...

>> I'm hand waving, the issue is that the extra state you extract from one
>> stream needs to be passed to the insert_stream() side of the equation.
>> Which is a fairly significant API break, so it should be done with some
>> care. (to avoid re-doing it lots of times.)
>
> I disagree here; we should preserve a strict pipelining model, which is
> what spiv and I have been working on. That permits e.g. forking a
> process locally and multi tasking, working with http post, etc.
>
> -Rob
>

Somehow you have to get the sha1s that you see while inserting inventory
pages to be passed to the code that is inserting the file texts.

The Source side needs to know the key references in order to send that
stream without having to have the Sink request the data.

However the Sink should be independently verifying that the stream it is
getting is valid. Arguably you should not double handle the data if
everything is in one process. But things being transmitted over the
network should be validated.

Perhaps the confusion is that I used "insert_stream" rather than
"insert_record_stream" in my example? (irs is just longer to type.)

There is a logical "get a stream of data from the server" which will be
a bunch of different types. As you read one type of data, you need to
accumulate some state about what you expect the next type of data to
look like. When you get that new data, you should compare it.

The Source side also does something similar. If we just forget about the
double-handling problem then it isn't a big deal. (One possibility would
be to have this configured based on the StreamSink, and an in-process
sink would know not to validate because the source has already done so.)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkpoAt0ACgkQJdeBCYSNAANsXACcC7o8ZN79H10Elq26eqDWjEVu
AlsAoJa7SsmztIFGYuXJ+gnWlE17cukK
=rBp2
-----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...

>> I'm hand waving, the issue is that the extra state you extract from one
>> stream needs to be passed to the insert_stream() side of the equation.
>> Which is a fairly significant API break, so it should be done with some
>> care. (to avoid re-doing it lots of times.)
> 
> I disagree here; we should preserve a strict pipelining model, which is
> what spiv and I have been working on. That permits e.g. forking a
> process locally and multi tasking, working with http post, etc.
> 
> -Rob
>