Hi Yufei, > I worry we’re introducing a protocol violation for a risk whose practical > impact we haven't established, while the possibility of silently incomplete > results is clear
I propose [1] to address the protocol compatibility issue by failing non-paginated requests whose response overflow the LIST_PAGINATION_MAX_PAGE_SIZE value (if configured). > why do we need the PR 5282? As for me, the rationale in the PR description is sufficient to justify the code change. I hope Ayush may be able to share some practical considerations that led to this PR. [1] https://lists.apache.org/thread/ntn71oh7g0kkf1wdhskh8t986gdwh1p3 Cheers, Dmitri. On Fri, Sep 18, 2026 at 7:21 PM Yufei Gu <[email protected]> wrote: > Hi Ayush, Dmitri, > > If option 1 isn't necessary, why do we need the PR 5282? Have we seen list > requests cause OOM in a real deployment, or reproduced this with a > realistic workload? > List responses contain names, not full table metadata. Let's use > uncompressed JSON payload sizes as a reference: > > Table-name length 10K tables 100K tables > 20 ASCII characters 0.55 MiB 5.53 MiB > 50 ASCII characters 0.84 MiB 8.39 MiB > 100 ASCII characters 1.32 MiB 13.16 MiB > > If the number of tables exceeds 100k under a single namespace, the system > may have problems beyond just the list operation, for example, the overlap > checking that can enumerate and resolve all siblings. > > I worry we’re introducing a protocol violation for a risk whose practical > impact we haven't established, while the possibility of silently incomplete > results is clear > > > Yufei > > > On Fri, Sep 18, 2026 at 8:43 AM Dmitri Bourlatchkov <[email protected]> > wrote: > > > Hi All, > > > > Many thanks to Ayush for referencing the earlier GH review discussion > [1]! > > > > From my POV, the main benefit of LIST_PAGINATION_MAX_PAGE_SIZE is capping > > _all_ responses to protect servers from overload. > > > > Even now, without any other config options, administrators are able to > > set LIST_PAGINATION_MAX_PAGE_SIZE to a negative value (default) thus > > enabling strict adherence to the IRC spec. > > > > If the risk of overload / OOM in the servers is substantial, the admin > can > > set LIST_PAGINATION_MAX_PAGE_SIZE to a positive value. This indeed will > be > > a deviation from the IRC spec. However, I think it is a > > reasonable deviation when overload is a risk. A protocol spec ought not > to > > force implementations into behaviours susceptible to DoS attacks or > general > > overload under normal circumstances. > > > > I do not see a rationale for enforcing LIST_PAGINATION_MAX_PAGE_SIZE > _only_ > > when the client expliocitly requests pagination. However, if other people > > prefer adding yet another flag, I think it would be fine as long as > > administrators can still apply global page size limits based on their > > deployment requirements. > > > > Another option is to error out of non-paginated requests > > when LIST_PAGINATION_MAX_PAGE_SIZE is in effect. This will maintain > strict > > IRC spec compatibility at the expense of failing risky requests. Perhaps > > this is the cleanest approach from the troubleshooting perspective. WDYT? > > > > [1] https://github.com/apache/polaris/pull/5282#discussion_r3768478185 > > > > Cheers, > > Dmitri. > > > > > > On Fri, Sep 18, 2026 at 8:01 AM Ayush Saxena <[email protected]> > > wrote: > > > > > Hi Yufei, > > > > > > Agreed that there's currently no way to get option 1 on its own — > that's > > a > > > fair gap. > > > > > > Option 2 isn't incidental though; It came out of the #5282 review [1], > > > where the concern was that a large catalog is otherwise exposed to OOM > > and > > > DoS through arbitrary unbounded list queries. A cap that exempts > requests > > > without pageToken doesn't bound that case at all — a client sending no > > > pagination parameters is precisely the one asking for every table in a > > > namespace, so the cap would only restrain clients that were already > > > paginating. > > > > > > That's why it's left to the operator and defaults to off: where clients > > > relying on a complete response are known to exist, it stays disabled as > > it > > > is today; where they are known not to exist, or supporting them isn't > > > intended, it can be turned on as added protection. > > > > > > Supporting both looks reasonable, with one more flag to choose between > > > them. If folks think we should add one more flag to toggle between > this, > > we > > > can add that. > > > > > > -Ayush > > > > > > [1] https://github.com/apache/polaris/pull/5282#discussion_r3768478185 > > > > > > On 2026/09/18 05:13:35 Yufei Gu wrote: > > > > Hi all, > > > > > > > > While reviewing PR #5533, I noticed that when a client doesn't send > > page > > > > token/size, the server can send partial result, which is a violation > of > > > IRC > > > > spec. It was enabled by the LIST_PAGINATION_MAX_PAGE_SIZE introduced > in > > > > #5282. I think it combines two behaviors: > > > > > > > > 1. Cap page sizes when the client explicitly opts into pagination > > via > > > > pageToken, while returning all results for requests without it. > This > > > > preserves the IRC contract. > > > > 2. Force pagination even when the client expects a complete > > response, > > > > deviating from IRC. > > > > > > > > Currently, setting LIST_PAGINATION_MAX_PAGE_SIZE enables both. > > Defaulting > > > > it to unlimited avoids changing defaults, but administrators still > > cannot > > > > choose option 1 alone. > > > > > > > > Option 1 is valid. Option 2 can silently produce incorrect results > for > > > IRC > > > > clients. I'm not sure how much value Option 2 provides. It may > > > potentially > > > > protect the server from OOM, I guess. Please chime in the use cases. > > > > > > > > If we really want to keep option 2, could we make it a separate, > > explicit > > > > setting? > > > > > > > > Thoughts? > > > > > > > > Related discussion: > > > > https://lists.apache.org/thread/mmqllcbt2lfsrq571cfwbymgfwoxqdf0 > > > > > > > > Yufei > > > > > > > > > >
