sudheerv edited a comment on pull request #6869: URL: https://github.com/apache/trafficserver/pull/6869#issuecomment-645617443
> Adding `proxy.config.hostdb.io.max_buffer_index` for #6850 looks reasonable, but other changes I have questions. > > 1. `iobuffer_size_to_index()` vs `buffer_size_to_index()` > I don't know why, but we have similar functions. IIUC, the difference is `iobuffer_size_to_index()` returns negative value if requested size is larger than `BUFFER_SIZE_FOR_INDEX(max)`. OTOH, `buffer_size_to_index()` returns `0` as `DEFAULT_BUFFER_BASE_SIZE` in such case. [Sudheer ] No, the idea is to enforce the caller pass the explicit buffer size as opposed to the iobuf library determining silently which could be problematic as it's hard to find a common default value that satisfies all use cases. > So we don't need to change `buffer_size_to_index()` calls if our target is `iobuffer_size_to_index()`. > 2. If we go this way, we should get rid of the default value of `iobuffer_size_to_index()` to make sure the caller sets the value explicitly. > > ``` > - extern int64_t iobuffer_size_to_index(int64_t size, int64_t max = max_iobuffer_size); > + extern int64_t iobuffer_size_to_index(int64_t size, int64_t max); > ``` > [Sudheer] +1. I removed it in my local patch that I'm testing in prod, but, forgot to include that change in the PR. Will add it now. > 1. Some `iobuffer_size_to_index()` calls has value from dedicated records.config, others has fixed value (`BUFFER_SIZE_INDEX_32K`). What's the policy of this difference? [Sudheer] Yeah, ideally we'd like all the use cases to have a config. I skipped QUIC use cases alone for the time being to revisit in the future when they are being used in prod. Do you recommend adding it to that as well? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
