<div dir="ltr"><div>Hi,</div><div><br></div><div>This stevedore API looks great, and I believe it would work very well</div><div>from the existing Varnish stevedore's point of view. But I have</div><div>difficulties seeing this work well from a design that relies on double</div><div>buffers. There might be details that I'm overlooking / not</div><div>understanding fully at this time, so I'll try to outline my concerns.</div><div><br></div><div>As I'm reading the API, Varnish will allow clients to stream also from</div><div>the interim storage segments. In a double-buffered scenario, this</div><div>means that the buffers can't be reused until all of the clients have</div><div>finished with them. This is possible to do using refcounting in the</div><div>ref()/rel() functions, but with a sufficiently popular object there</div><div>will be slow clients pinning the buffers effectively doubling the</div><div>memory pressure of a fetch.</div><div><br></div><div>In my opinion the API needs to allow for the stevedore to be able to</div><div>do double buffering using a single buffer that is being reused for</div><div>each call to alloc(). This means that no streaming client should be</div><div>allowed to touch any data until after the commit() call has been</div><div>made, which should remove the need for the interrim seg_id's.</div><div><br></div><div>During the IRC discussions on this, it was argued that it would be</div><div>possible to have a buffer size returned from alloc() that is smaller</div><div>than the size of the segment it buffers for. The real segment will</div><div>then be appended to on each commit(), and the seg_id returned from</div><div>commit() will then just be repeated for each append.</div><div><br></div><div>I find this approach lacking. First, there will be no opportunity to</div><div>coalesche the seg_id's. This can be handled in the stevedore during</div><div>segment lookups (the ref() API call), by returning 0 lengths on all</div><div>but the first ref() for the same seg_id by each client. This ofc then</div><div>requires per client state in the stevedore. Also the seg_id list</div><div>becomes unnecessarily big wasting space.</div><div><br></div><div>Secondly, since the commit() function is supposed to be the point</div><div>where trim() functionality happens, I fail to see how the code should</div><div>be able to distinguish a commit() as an append from a commit() where a</div><div>trim is needed.</div><div><br></div><div><br></div><div>Based on this, I have an slightly modified suggestion for the API:</div><div><br></div><div><br></div><div>New objects are created with stv->newobj():</div><div><br></div><div>* Arguments:</div><div> * busyobj</div><div> * total size estimate</div><div>* Returns:</div><div> * yes/no</div><div><br></div><div>busyobj/objcore has private field(s) for the stevedores private use</div><div>from here on.</div><div><br></div><div>The stevedore can fail this if the object is undesirable, and another</div><div>stevedore will be attempted, if nothing else, Transient.</div><div> </div><div><br></div><div>Storage segments are allocated with stv->alloc():</div><div><br></div><div>* Arguments:</div><div> * busyobj</div><div> * desired number of bytes</div><div> * (uintptr_t) seg_id or 0.</div><div>* Returns:</div><div> * (void *) priv</div><div> * data pointer</div><div> * length</div><div><br></div><div>The calling code can supply a previous seg_id that it would be OK to</div><div>expand if the stevedore is able to (e.g. there is more room in the</div><div>segment). This would typically be the previous seg_id received from</div><div>commit(). Each call to alloc() needs to be followed by a commit(),</div><div>passing the priv received. alloc() can fail, and indicates failure by</div><div>returning a NULL data pointer.</div><div><br></div><div><br></div><div>Once the segment has been written to: stv->commit()</div><div><br></div><div>* Arguments:</div><div> * busyobj</div><div> * (void *) priv</div><div> * length used</div><div>* Returns:</div><div> * (uintptr_t) seg_id</div><div><br></div><div>This commits the storage and returns a seg_id(). This could be a new</div><div>seg_id, or the same as the one passed to alloc() in which case it</div><div>should not be added to the seg_id() list forming the object. commit()</div><div>can fail, and indicates failure by returning a 0 seg_id.</div><div><br></div><div><br></div><div>Trimming a segment: stv->trim()</div><div><br></div><div>* Arguments:</div><div> * busyobj</div><div> * (uintptr_t) seg_id</div><div>* Returns:</div><div> * (uintptr_t) seg_id</div><div><br></div><div>The calling code typically calls this at the end of a fetch</div><div>operation. It can return a seg_id that is to replace the one passed,</div><div>which the calling code should free() when it can ensure there are no</div><div>readers (e.g. busyobj destruction). A trim that doesn't change the</div><div>semantics (e.g. -sfile) will return 0 meaning no replace is necessary.</div><div><br></div><div><br></div><div>Object is finalized with stv->final():</div><div><br></div><div>* Arguments:</div><div> * busyobj (not NULL)</div><div> * seg_id (key to resurrect persistent object)</div><div> * off_t (subkey to resurrect persistent object)</div><div>* Returns:</div><div> none (failure point?)</div><div><br></div><div><br></div><div>Objects are killd with stv->deleteobj():</div><div><br></div><div>* Arguments:</div><div> * objcore</div><div><br></div><div><br></div><div>All allocated segments are individually freed with stv->free():</div><div><br></div><div>* Arguments:</div><div> * (uintptr_t) seg_id</div><div><br></div><div><br></div><div>Regards,</div><div>Martin Blix Grydeland</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 13 November 2014 10:02, Poul-Henning Kamp <span dir="ltr"><<a href="mailto:phk@phk.freebsd.dk" target="_blank">phk@phk.freebsd.dk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I've been mucking about with the stevedore api for some days in order<br>
to resolve a number of silly issues, including the vast overallocation<br>
for tiny objects when streaming.<br>
<br>
There are suprisingly many dead ends in this area.<br>
<br>
I think I have finally managed to come up with something that is usable,<br>
both from stevedore and varnishd side.<br>
<br>
The crucial insight is that we do not need to store the OA first<br>
since we now have accessor functions. Instead we can collect the<br>
OAs in the busyobj until we know their final size, and only then<br>
allocate space for them.<br>
<br>
But we still need to alert persistent stevedores before we make the<br>
first allocation for a new object, and we also (which is new!) want<br>
to give stevedores a chance to relocate segments once their size is<br>
known for sure. In that case it's the stevedores responsibility<br>
to keep track of streamers of the interrim segment before freeing it.<br>
<br>
Persistent stevedores also must be told what "magic key" it must<br>
present to resurrect an object.<br>
<br>
And finally we make it the cache_obj.c codes responsibility to free<br>
all allocated segments individually, so that the stevedore does not<br>
need to keep track of that.<br>
<br>
Seen from the stevedore:<br>
<br>
New objects are created with stv->newobj().<br>
<br>
Arguments:<br>
busyobj<br>
total size estimate<br>
Returns:<br>
yes/no<br>
<br>
busyobj/objcore has private field(s) for the stevedores<br>
private use from here on.<br>
<br>
The stevedore can fail this if the object is undesirable,<br>
and another stevedore will be attempted, if nothing else, Transient.<br>
<br>
Storage segments are allocated with stv->alloc()<br>
Arguments:<br>
busyobj<br>
desired number of bytes<br>
Returns:<br>
interrim seg_id (uintptr_t)<br>
data pointer<br>
length<br>
<br>
Once the segment has been written to: stv->commit()<br>
Arguments:<br>
busyobj<br>
interrim seg_id<br>
length used<br>
Returns:<br>
final seg_id, may be different from interim seg_id<br>
<br>
Other threads may reference segments with stv->ref()<br>
Arguments:<br>
busyobj (possibly NULL)<br>
seg_id (interrim or final)<br>
Returns:<br>
data pointer<br>
length<br>
<br>
References are released with stv->rel()<br>
Arguments:<br>
busyobj (possibly NULL)<br>
seg_id (interrim or final)<br>
Returns:<br>
none<br>
<br>
Object is finalized with stv->final()<br>
Arguments:<br>
busyobj (not NULL)<br>
seg_id (key to resurrect persistent object)<br>
off_t (subkey to resurrect persistent object)<br>
Returns:<br>
none<br>
<br>
Objects are killed with stv->deleteobj()<br>
Arguments:<br>
objcore<br>
<br>
All allocated segments are individually freed with stv->free()<br>
Arguments:<br>
seg_id<br>
<br>
Seen from varnishd:<br>
<br>
stv->newobj()<br>
collect OA's in busyobj and access from there<br>
stv->alloc/commit until body is received<br>
all OA's defined.<br>
byte-stream encode OAs into last storage segment<br>
(including OA listing storage segment id's)<br>
(if space allows) else into separate segment<br>
stv->final(coords for OA bytestream)<br>
OA's now pulled out of stored object<br>
<br>
Comments most welcome...<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Poul-Henning Kamp | UNIX since Zilog Zeus 3.20<br>
phk@FreeBSD.ORG | TCP/IP since RFC 956<br>
FreeBSD committer | BSD since 4.3-tahoe<br>
Never attribute to malice what can adequately be explained by incompetence.<br>
<br>
_______________________________________________<br>
varnish-dev mailing list<br>
<a href="mailto:varnish-dev@varnish-cache.org">varnish-dev@varnish-cache.org</a><br>
<a href="https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev" target="_blank">https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev</a><br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div><div><table border="0" cellpadding="0" cellspacing="0" style="font-size:12px;line-height:1.5em;font-family:'Helvetica Neue',Arial,sans-serif;color:rgb(102,102,102);width:550px;border-top-width:1px;border-top-style:solid;border-top-color:rgb(238,238,238);border-bottom-width:1px;border-bottom-style:solid;border-bottom-color:rgb(238,238,238);margin-top:20px;padding-top:5px;padding-bottom:5px"><tbody><tr><td width="100"><a href="http://varnish-software.com" target="_blank"><img src="http://www.varnish-software.com/static/media/logo-email.png"></a><span></span><span></span></td><td><strong style="font-size:14px;color:rgb(34,34,34)">Martin Blix Grydeland</strong><br>Senior Developer | Varnish Software AS<br>Mobile: +47 992 74 756<br><span style="font-weight:bold">We Make Websites Fly!</span></td></tr></tbody></table></div></div></div></div>
</div>