Re: [PATCH v2 04/11] upload-pack: advertise trace2 SID in v0 capabilities

2 messages, 2 authors, 2020-11-05 · open the first message on its own page

Re: [PATCH v2 04/11] upload-pack: advertise trace2 SID in v0 capabilities

From: Junio C Hamano <hidden>
Date: 2020-11-03 21:48:32

Josh Steadmon [off-list ref] writes:
When trace2 is enabled and trace2.advertiseSID is true, advertise
upload-pack's trace2 session ID via the new trace2-sid capability.
I would have imagined when advertiseSID is enabled, trace2, at least
the part that allocates and assigns the session ID, ought to be
enabled automatically.

But the above goes in a different direction and requires both to be
enabled.  Any compelling reason behind the choice?

Does the documentation added by this series make it clear that
asking for advertiseSID does NOT automatically enable allocation of
session IDs (even if it does not explain why it does not happen)?

Thanks.

Re: [PATCH v2 04/11] upload-pack: advertise trace2 SID in v0 capabilities

From: Josh Steadmon <hidden>
Date: 2020-11-05 18:45:04

On 2020.11.03 13:48, Junio C Hamano wrote:
Josh Steadmon [off-list ref] writes:
quoted
When trace2 is enabled and trace2.advertiseSID is true, advertise
upload-pack's trace2 session ID via the new trace2-sid capability.
I would have imagined when advertiseSID is enabled, trace2, at least
the part that allocates and assigns the session ID, ought to be
enabled automatically.

But the above goes in a different direction and requires both to be
enabled.  Any compelling reason behind the choice?
My reasoning was that by advertising the capability, you are telling the
remote side "I have definitely produced a log using this session ID. If
you need it later, you can find it with this key". If we advertise a
session ID even when trace2 is not enabled, the remote side can't be as
sure that the received session ID actually points to any useful logs on
the other side.

Of course, this is a weak guarantee since a client could send whatever
it likes regardless of whether anything was logged, or one side could
delete or lose its logs before the other decides it needs to view them.

I think your idea in a different subthread about having a general
session ID not tied to trace2 is interesting, and would also be a point
in favor of changing the current behavior here, but I have some thoughts
on that point that I'll add in the other subthread.

I'm still leaning towards advertising a session ID only if we actually
produced logs locally, but I'm open to further discussion.
Does the documentation added by this series make it clear that
asking for advertiseSID does NOT automatically enable allocation of
session IDs (even if it does not explain why it does not happen)?
In V3 I'll update the docs to call out whichever decision we reach on
this point.
Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help