Also, In addition to a new API TSFetchFlagSet to set the fetch flags, also need to change the return type for TSFetchUrl() to return the FetchSM context.
TSFetchSM TSFetchUrl(const char *headers, int request_len, sockaddr const *ip, TSCont contp, TSFetchWakeUpOptions callback_options, TSFetchEvent events) > +void > +TSFetchFlagSet(TSFetchSM fetch_sm, int flags) > On Mar 6, 2020, at 8:40 AM, Sudheer Vinukonda <sudheervinuko...@yahoo.com> > wrote: > > > Sorry, I just realized that TSFetchUrl() can be called directly without > calling TSFetchCreate(). So, to preserve backward compatibility and to be > able to set the fetch flags, I'm going to add a new `TSFetchFlagSet` API as > below. > > The API accepts a previously created FetchSM context and the flags to be set > on it. > > > +void > +TSFetchFlagSet(TSFetchSM fetch_sm, int flags) > +{ > + sdk_assert(sdk_sanity_check_fetch_sm(fetch_sm) == TS_SUCCESS); > + (reinterpret_cast<FetchSM *>(fetch_sm))->set_fetch_flags(flags); > +} > + > > > Since this is a new API, I'll wait for a few more days for the review. > > > > > > On Friday, March 6, 2020, 08:03:03 AM PST, Sudheer Vinukonda > <sudheervinuko...@yahoo.com> wrote: > > > I'm going to call this in and make the change to the TSFetchUrl() API to > remove setting `TS_FETCH_FLAGS_DECHUNK` flag by default. > > After discussing with Leif over slack, it's worth explicitly clarifying that > plugins that do want to still have `TS_FETCH_FLAGS_DECHUNK` set can do so by > simply passing the flag during `TSFetchCreate()` explicitly (that is exactly > what we are going to do in our plugins). > > > -- Sudheer > > > > > > On Wednesday, March 4, 2020, 10:06:00 AM PST, Sudheer Vinukonda > <sudheervinuko...@yahoo.com.invalid> wrote: > > > We use TSFetchUrl() API in a bunch of our use cases to trigger sideways > calls. The API by default sets `TS_FETCH_FLAGS_DECHUNK`,which implies the > response returned is dechunked. There's another flag called > 'TS_FETCH_FLAGS_STREAM' in FechSM, which is NOT set by default. > This makes TSFetchUrl API a bit conflating and contradictory since dechunking > is only done when 'TS_FETCH_FLAGS_STREAM' is set ignoring the > `TS_FETCH_FLAGS_DECHUNK`. > Our plugins can not handle chunked responses on their own and we don't want > TS_FETCH_FLAGS_STREAM. We just want the entire response (headers + body) to > be returned together set by the callback_options AFTER_BODY (similar to how > esi plugin works). > The side-effect of all of this is that we can not use HTTP/1.1 to the Origins > return chunked responses and the contradictory flags above mean that our > plugins end up getting chunked body (with chunk sizes, trailers etc). > I'd like to change the default TSFetchUrl() API mode to not set > `TS_FETCH_FLAGS_DECHUNK` anymore and let callers ask explicitly if they want > dechunked body similar to how 'TS_FETCH_FLAGS_STREAM' flag works. > The resultant behavior would be that a plugin would get chunked body only > when it does not set ` 'TS_FETCH_FLAGS_STREAM' ` and `TS_FETCH_FLAGS_DECHUNK` > explicitly. If it does either, it'd get dechunked body (streamed in the first > case and non-streamed in the latter). > This will make the flags consistent and preserves the backward compatibility > that esi plugin needs as it does not set `TS_FETCH_FLAGS_STREAM` (and > `TS_FETCH_FLAGS_DECHUNK`) explicitly. We will update our plugins to > explicitly ask for `TS_FETCH_FLAGS_DECHUNK`. > Let me know if there are any concerns/suggestions.