You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@polaris.apache.org by Yufei Gu <fl...@gmail.com> on 2026/09/18 05:13:35 UTC
[DISCUSS] Separate list page-size limits from forced pagination
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
Re: [DISCUSS] Separate list page-size limits from forced pagination
Posted by Yufei Gu <fl...@gmail.com>.
Erroring out for the incomplete list sounds good to me. Thanks for the
discussion!
Yufei
On Sat, Sep 19, 2026 at 9:00 PM Ayush Saxena <ay...@apache.org> wrote:
> Thanx Everyone for the feedback so far. I have created a PR to reject in
> case of overflow [1], in case the discussion drifts in different direction,
> will update accordingly.
>
> -Ayush
>
> [1] https://github.com/apache/polaris/pull/5553
>
> On 2026/09/19 12:35:24 vignesh a wrote:
> > Hi all,
> >
> > +1 to fail-closed on the existing LIST_PAGINATION_MAX_PAGE_SIZE, and I
> > don't think we need another flag.
> >
> > A truncated 200 on a request that never asked to page is the bad
> > outcome. Rejecting when the unsized result would exceed the configured
> > maximum is the contract that stays diagnosable: paginated requests are
> > capped, an unsized request that fits is complete, one that would
> > overflow fails instead of looking complete.
> >
> > Prithvi's two details have to land in the follow-up or we only fix the
> > client lie:
> >
> > 1. Probe at max+1 and reject if more remain. Listing everything into
> > heap and then throwing still pays the cost Ayush described.
> >
> > 2. Missing pageToken is unsized. Empty pageToken is pagination. Java
> > Iceberg sends pageToken= and will follow next-page-token;
> PageTokenUtil
> > currently treats null and "" the same, and that has to change for
> > reject or we 400 a client that was already prepared to page.
> >
> > Federated listing has the same silent truncation today (null pageToken
> > becomes the initial empty token when the max is set). Reject has to
> > apply there too; CatalogHandlerUtils still materializes the full remote
> > list before slicing, so max+1 on that path is a separate follow-up.
> >
> > Default stays off.
> >
> > Thanks,
> > Vignesh
> >
> > On Sat, 19 Sept 2026 at 15:10, Prithvi S <pr...@gmail.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > Thank you for laying this out so clearly. Yufei is right that the
> current
> > > on-path behaviour is the problem worth fixing: a 200 with a
> continuation
> > > token, on a request that never asked for pagination, is a silently
> > > incomplete list. That is worse for catalog correctness than a loud
> failure.
> > > Robert already flagged the same class of client on #5282 (PyIceberg
> 0.10 /
> > > 0.11 does one unparameterized list and returns it). #5533 shows it
> closer
> > > to home: the generic-tables CLI follows next-page-token only when
> > > --page-size is set, so a configured maximum would drop the rest of the
> > > tables with no error.
> > > I don't think we need a second flag.
> > > Option 1 (cap an explicit pageSize, leave unsized requests complete)
> is the
> > > original #5282 ceiling, and it is Iceberg-legal: pageSize is an upper
> > > bound. It is not server protection. A client that sends no pageToken
> is the
> > > one asking for every identifier under the namespace; exempting that
> request
> > > leaves the unbounded case untouched. Option 2 (force pagination on that
> > > request) does bound it, which is why the review grew in that
> direction, but
> > > returning a partial 200 is the wrong shape for that bound.
> > >
> > > Dmitri's fail-closed rule, is the contract I would want from the
> existing
> > > setting:
> > > • paginated request: cap to LIST_PAGINATION_MAX_PAGE_SIZE
> > > • unsized request that fits: complete list, null next-page-token
> > > • unsized request that would exceed: reject, not a truncated 200
> > > Default stays -1, so nothing changes for deployments that have not
> opted
> > > in. LIST_PAGINATION_ENABLED=false remains the escape hatch that ignores
> > > pagination parameters. Operators who need a ceiling turn it on and get
> a
> > > bound they can state and diagnose. Clients that believe they hold a
> > > complete list either do, or they get an error telling them to paginate
> or
> > > raise the limit.
> > >
> > > Two implementation details I think the follow-up PR has to get right,
> or
> > > reject will not do what we are claiming.
> > > 1. Reject has to stop at max+1, not list everything and then throw.
> > > Page.mapped already stops after pageSize items when pagination is
> > > requested, and the JDBC listing path has LIMIT. If the unsized path
> still
> > > materializes the full result into a List, copies it into the response,
> and
> > > serializes it, we pay the heap and concurrency cost Ayush described and
> > > then fail. That protects the client from a lie; it does not protect the
> > > server. Probe with size max+1; if a continuation would have been
> produced,
> > > return 400 with the configured maximum in the message. The federated
> path
> > > needs the same rule. Today, when the maximum is set and pageToken is
> null,
> > > IcebergCatalogHandler injects INITIAL_PAGE_TOKEN so the remote listing
> is
> > > sliced in memory. That is the same silent truncation. Request at most
> max+1
> > > from the remote catalog and reject if more remain.
> > > 2. Distinguish a missing pageToken from an empty one. The Iceberg REST
> spec
> > > is explicit: clients start pagination by sending an empty pageToken;
> if the
> > > parameter is not set, the server must return the complete result. Java
> > > Iceberg always sends pageToken= and follows next-page-token even when
> > > rest-page-size is unset. PageTokenUtil currently treats null and "" the
> > > same, which is harmless for force-pagination and harmful for reject: a
> > > naive "unsized means reject if over max" would 400 the Java client
> that was
> > > already prepared to page. Reject only when pageToken is absent (and
> > > pageSize is absent). Empty pageToken, or an explicit pageSize, is
> > > pagination and should be capped.
> > >
> > > On the overload question: Yufei's uncompressed JSON sizes are right
> for the
> > > wire. I have not seen a published listTables OOM either. I still think
> a
> > > default-off ceiling is a reasonable operator tool, peak heap is several
> > > in-memory copies, held for the duration of the response, multiplied by
> > > concurrency, and federation currently pulls the remote list into
> Polaris
> > > before slicing it. We don't need a production incident to keep the
> knob. We
> > > do need the knob not to return a 200 that looks complete and is not.
> > >
> > > Thanks,
> > > Prithvi S
> > >
> > > On Sat, Sep 19, 2026 at 7:02 AM Ayush Saxena <ay...@apache.org>
> > > wrote:
> > >
> > > > Hi Yufei, Dmitri,
> > > > Thanks both for working through this.
> > > >
> > > > I'm inclined towards Dmitri's suggestion of throwing an exception. It
> > > > gives a contract that is easy to state and easy to diagnose: when a
> > > > maximum is configured, that value is the bound the server will
> honour.
> > > > A paginated request is capped to it; an unpaged request that fits
> still
> > > > gets the complete list; an unpaged request that would exceed it is
> > > > rejected rather than quietly truncated. A client that believes it
> holds
> > > > a complete list when it does not is the failure worth designing
> against.
> > > > If that sounds reasonable, I can raise a PR for it.
> > > >
> > > > On the overload question, I don't think the per-response payload is
> the
> > > > whole picture. For a request without pagination the full result set
> is
> > > > materialized in heap before anything is written — Page.mapped
> collects
> > > > every row into a list, the handler copies it into the response
> > > > collection, and it is then serialized — so peak memory is a multiple
> of
> > > > the figures in the table, and it is held for the duration of the
> > > > response rather than streamed. That cost also multiplies with
> > > > concurrency rather than with a single listing, and a large response
> > > > occupies the connection while it is written, so a handful of
> > > > simultaneous unbounded lists is a different proposition from one. A
> > > > configurable ceiling is what lets an operator put a bound on that.
> > > >
> > > > For context on where the flag came from: it started with a review
> > > > comment [1].
> > > >
> > > > -Ayush
> > > > [1]
> https://github.com/apache/polaris/pull/5255#discussion_r3739619320
> > > >
> > > > On 2026/09/19 00:02:17 Dmitri Bourlatchkov wrote:
> > > > > 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 <fl...@gmail.com>
> 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 <
> > > dimas@apache.org>
> > > > > > 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 <
> > > ayushsaxena@apache.org
> > > > >
> > > > > > > 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
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Separate list page-size limits from forced pagination
Posted by Ayush Saxena <ay...@apache.org>.
Thanx Everyone for the feedback so far. I have created a PR to reject in case of overflow [1], in case the discussion drifts in different direction, will update accordingly.
-Ayush
[1] https://github.com/apache/polaris/pull/5553
On 2026/09/19 12:35:24 vignesh a wrote:
> Hi all,
>
> +1 to fail-closed on the existing LIST_PAGINATION_MAX_PAGE_SIZE, and I
> don't think we need another flag.
>
> A truncated 200 on a request that never asked to page is the bad
> outcome. Rejecting when the unsized result would exceed the configured
> maximum is the contract that stays diagnosable: paginated requests are
> capped, an unsized request that fits is complete, one that would
> overflow fails instead of looking complete.
>
> Prithvi's two details have to land in the follow-up or we only fix the
> client lie:
>
> 1. Probe at max+1 and reject if more remain. Listing everything into
> heap and then throwing still pays the cost Ayush described.
>
> 2. Missing pageToken is unsized. Empty pageToken is pagination. Java
> Iceberg sends pageToken= and will follow next-page-token; PageTokenUtil
> currently treats null and "" the same, and that has to change for
> reject or we 400 a client that was already prepared to page.
>
> Federated listing has the same silent truncation today (null pageToken
> becomes the initial empty token when the max is set). Reject has to
> apply there too; CatalogHandlerUtils still materializes the full remote
> list before slicing, so max+1 on that path is a separate follow-up.
>
> Default stays off.
>
> Thanks,
> Vignesh
>
> On Sat, 19 Sept 2026 at 15:10, Prithvi S <pr...@gmail.com>
> wrote:
>
> > Hi all,
> >
> > Thank you for laying this out so clearly. Yufei is right that the current
> > on-path behaviour is the problem worth fixing: a 200 with a continuation
> > token, on a request that never asked for pagination, is a silently
> > incomplete list. That is worse for catalog correctness than a loud failure.
> > Robert already flagged the same class of client on #5282 (PyIceberg 0.10 /
> > 0.11 does one unparameterized list and returns it). #5533 shows it closer
> > to home: the generic-tables CLI follows next-page-token only when
> > --page-size is set, so a configured maximum would drop the rest of the
> > tables with no error.
> > I don't think we need a second flag.
> > Option 1 (cap an explicit pageSize, leave unsized requests complete) is the
> > original #5282 ceiling, and it is Iceberg-legal: pageSize is an upper
> > bound. It is not server protection. A client that sends no pageToken is the
> > one asking for every identifier under the namespace; exempting that request
> > leaves the unbounded case untouched. Option 2 (force pagination on that
> > request) does bound it, which is why the review grew in that direction, but
> > returning a partial 200 is the wrong shape for that bound.
> >
> > Dmitri's fail-closed rule, is the contract I would want from the existing
> > setting:
> > • paginated request: cap to LIST_PAGINATION_MAX_PAGE_SIZE
> > • unsized request that fits: complete list, null next-page-token
> > • unsized request that would exceed: reject, not a truncated 200
> > Default stays -1, so nothing changes for deployments that have not opted
> > in. LIST_PAGINATION_ENABLED=false remains the escape hatch that ignores
> > pagination parameters. Operators who need a ceiling turn it on and get a
> > bound they can state and diagnose. Clients that believe they hold a
> > complete list either do, or they get an error telling them to paginate or
> > raise the limit.
> >
> > Two implementation details I think the follow-up PR has to get right, or
> > reject will not do what we are claiming.
> > 1. Reject has to stop at max+1, not list everything and then throw.
> > Page.mapped already stops after pageSize items when pagination is
> > requested, and the JDBC listing path has LIMIT. If the unsized path still
> > materializes the full result into a List, copies it into the response, and
> > serializes it, we pay the heap and concurrency cost Ayush described and
> > then fail. That protects the client from a lie; it does not protect the
> > server. Probe with size max+1; if a continuation would have been produced,
> > return 400 with the configured maximum in the message. The federated path
> > needs the same rule. Today, when the maximum is set and pageToken is null,
> > IcebergCatalogHandler injects INITIAL_PAGE_TOKEN so the remote listing is
> > sliced in memory. That is the same silent truncation. Request at most max+1
> > from the remote catalog and reject if more remain.
> > 2. Distinguish a missing pageToken from an empty one. The Iceberg REST spec
> > is explicit: clients start pagination by sending an empty pageToken; if the
> > parameter is not set, the server must return the complete result. Java
> > Iceberg always sends pageToken= and follows next-page-token even when
> > rest-page-size is unset. PageTokenUtil currently treats null and "" the
> > same, which is harmless for force-pagination and harmful for reject: a
> > naive "unsized means reject if over max" would 400 the Java client that was
> > already prepared to page. Reject only when pageToken is absent (and
> > pageSize is absent). Empty pageToken, or an explicit pageSize, is
> > pagination and should be capped.
> >
> > On the overload question: Yufei's uncompressed JSON sizes are right for the
> > wire. I have not seen a published listTables OOM either. I still think a
> > default-off ceiling is a reasonable operator tool, peak heap is several
> > in-memory copies, held for the duration of the response, multiplied by
> > concurrency, and federation currently pulls the remote list into Polaris
> > before slicing it. We don't need a production incident to keep the knob. We
> > do need the knob not to return a 200 that looks complete and is not.
> >
> > Thanks,
> > Prithvi S
> >
> > On Sat, Sep 19, 2026 at 7:02 AM Ayush Saxena <ay...@apache.org>
> > wrote:
> >
> > > Hi Yufei, Dmitri,
> > > Thanks both for working through this.
> > >
> > > I'm inclined towards Dmitri's suggestion of throwing an exception. It
> > > gives a contract that is easy to state and easy to diagnose: when a
> > > maximum is configured, that value is the bound the server will honour.
> > > A paginated request is capped to it; an unpaged request that fits still
> > > gets the complete list; an unpaged request that would exceed it is
> > > rejected rather than quietly truncated. A client that believes it holds
> > > a complete list when it does not is the failure worth designing against.
> > > If that sounds reasonable, I can raise a PR for it.
> > >
> > > On the overload question, I don't think the per-response payload is the
> > > whole picture. For a request without pagination the full result set is
> > > materialized in heap before anything is written — Page.mapped collects
> > > every row into a list, the handler copies it into the response
> > > collection, and it is then serialized — so peak memory is a multiple of
> > > the figures in the table, and it is held for the duration of the
> > > response rather than streamed. That cost also multiplies with
> > > concurrency rather than with a single listing, and a large response
> > > occupies the connection while it is written, so a handful of
> > > simultaneous unbounded lists is a different proposition from one. A
> > > configurable ceiling is what lets an operator put a bound on that.
> > >
> > > For context on where the flag came from: it started with a review
> > > comment [1].
> > >
> > > -Ayush
> > > [1] https://github.com/apache/polaris/pull/5255#discussion_r3739619320
> > >
> > > On 2026/09/19 00:02:17 Dmitri Bourlatchkov wrote:
> > > > 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 <fl...@gmail.com> 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 <
> > dimas@apache.org>
> > > > > 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 <
> > ayushsaxena@apache.org
> > > >
> > > > > > 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
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Separate list page-size limits from forced pagination
Posted by vignesh a <im...@gmail.com>.
Hi all,
+1 to fail-closed on the existing LIST_PAGINATION_MAX_PAGE_SIZE, and I
don't think we need another flag.
A truncated 200 on a request that never asked to page is the bad
outcome. Rejecting when the unsized result would exceed the configured
maximum is the contract that stays diagnosable: paginated requests are
capped, an unsized request that fits is complete, one that would
overflow fails instead of looking complete.
Prithvi's two details have to land in the follow-up or we only fix the
client lie:
1. Probe at max+1 and reject if more remain. Listing everything into
heap and then throwing still pays the cost Ayush described.
2. Missing pageToken is unsized. Empty pageToken is pagination. Java
Iceberg sends pageToken= and will follow next-page-token; PageTokenUtil
currently treats null and "" the same, and that has to change for
reject or we 400 a client that was already prepared to page.
Federated listing has the same silent truncation today (null pageToken
becomes the initial empty token when the max is set). Reject has to
apply there too; CatalogHandlerUtils still materializes the full remote
list before slicing, so max+1 on that path is a separate follow-up.
Default stays off.
Thanks,
Vignesh
On Sat, 19 Sept 2026 at 15:10, Prithvi S <pr...@gmail.com>
wrote:
> Hi all,
>
> Thank you for laying this out so clearly. Yufei is right that the current
> on-path behaviour is the problem worth fixing: a 200 with a continuation
> token, on a request that never asked for pagination, is a silently
> incomplete list. That is worse for catalog correctness than a loud failure.
> Robert already flagged the same class of client on #5282 (PyIceberg 0.10 /
> 0.11 does one unparameterized list and returns it). #5533 shows it closer
> to home: the generic-tables CLI follows next-page-token only when
> --page-size is set, so a configured maximum would drop the rest of the
> tables with no error.
> I don't think we need a second flag.
> Option 1 (cap an explicit pageSize, leave unsized requests complete) is the
> original #5282 ceiling, and it is Iceberg-legal: pageSize is an upper
> bound. It is not server protection. A client that sends no pageToken is the
> one asking for every identifier under the namespace; exempting that request
> leaves the unbounded case untouched. Option 2 (force pagination on that
> request) does bound it, which is why the review grew in that direction, but
> returning a partial 200 is the wrong shape for that bound.
>
> Dmitri's fail-closed rule, is the contract I would want from the existing
> setting:
> • paginated request: cap to LIST_PAGINATION_MAX_PAGE_SIZE
> • unsized request that fits: complete list, null next-page-token
> • unsized request that would exceed: reject, not a truncated 200
> Default stays -1, so nothing changes for deployments that have not opted
> in. LIST_PAGINATION_ENABLED=false remains the escape hatch that ignores
> pagination parameters. Operators who need a ceiling turn it on and get a
> bound they can state and diagnose. Clients that believe they hold a
> complete list either do, or they get an error telling them to paginate or
> raise the limit.
>
> Two implementation details I think the follow-up PR has to get right, or
> reject will not do what we are claiming.
> 1. Reject has to stop at max+1, not list everything and then throw.
> Page.mapped already stops after pageSize items when pagination is
> requested, and the JDBC listing path has LIMIT. If the unsized path still
> materializes the full result into a List, copies it into the response, and
> serializes it, we pay the heap and concurrency cost Ayush described and
> then fail. That protects the client from a lie; it does not protect the
> server. Probe with size max+1; if a continuation would have been produced,
> return 400 with the configured maximum in the message. The federated path
> needs the same rule. Today, when the maximum is set and pageToken is null,
> IcebergCatalogHandler injects INITIAL_PAGE_TOKEN so the remote listing is
> sliced in memory. That is the same silent truncation. Request at most max+1
> from the remote catalog and reject if more remain.
> 2. Distinguish a missing pageToken from an empty one. The Iceberg REST spec
> is explicit: clients start pagination by sending an empty pageToken; if the
> parameter is not set, the server must return the complete result. Java
> Iceberg always sends pageToken= and follows next-page-token even when
> rest-page-size is unset. PageTokenUtil currently treats null and "" the
> same, which is harmless for force-pagination and harmful for reject: a
> naive "unsized means reject if over max" would 400 the Java client that was
> already prepared to page. Reject only when pageToken is absent (and
> pageSize is absent). Empty pageToken, or an explicit pageSize, is
> pagination and should be capped.
>
> On the overload question: Yufei's uncompressed JSON sizes are right for the
> wire. I have not seen a published listTables OOM either. I still think a
> default-off ceiling is a reasonable operator tool, peak heap is several
> in-memory copies, held for the duration of the response, multiplied by
> concurrency, and federation currently pulls the remote list into Polaris
> before slicing it. We don't need a production incident to keep the knob. We
> do need the knob not to return a 200 that looks complete and is not.
>
> Thanks,
> Prithvi S
>
> On Sat, Sep 19, 2026 at 7:02 AM Ayush Saxena <ay...@apache.org>
> wrote:
>
> > Hi Yufei, Dmitri,
> > Thanks both for working through this.
> >
> > I'm inclined towards Dmitri's suggestion of throwing an exception. It
> > gives a contract that is easy to state and easy to diagnose: when a
> > maximum is configured, that value is the bound the server will honour.
> > A paginated request is capped to it; an unpaged request that fits still
> > gets the complete list; an unpaged request that would exceed it is
> > rejected rather than quietly truncated. A client that believes it holds
> > a complete list when it does not is the failure worth designing against.
> > If that sounds reasonable, I can raise a PR for it.
> >
> > On the overload question, I don't think the per-response payload is the
> > whole picture. For a request without pagination the full result set is
> > materialized in heap before anything is written — Page.mapped collects
> > every row into a list, the handler copies it into the response
> > collection, and it is then serialized — so peak memory is a multiple of
> > the figures in the table, and it is held for the duration of the
> > response rather than streamed. That cost also multiplies with
> > concurrency rather than with a single listing, and a large response
> > occupies the connection while it is written, so a handful of
> > simultaneous unbounded lists is a different proposition from one. A
> > configurable ceiling is what lets an operator put a bound on that.
> >
> > For context on where the flag came from: it started with a review
> > comment [1].
> >
> > -Ayush
> > [1] https://github.com/apache/polaris/pull/5255#discussion_r3739619320
> >
> > On 2026/09/19 00:02:17 Dmitri Bourlatchkov wrote:
> > > 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 <fl...@gmail.com> 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 <
> dimas@apache.org>
> > > > 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 <
> ayushsaxena@apache.org
> > >
> > > > > 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
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Separate list page-size limits from forced pagination
Posted by Prithvi S <pr...@gmail.com>.
Hi all,
Thank you for laying this out so clearly. Yufei is right that the current
on-path behaviour is the problem worth fixing: a 200 with a continuation
token, on a request that never asked for pagination, is a silently
incomplete list. That is worse for catalog correctness than a loud failure.
Robert already flagged the same class of client on #5282 (PyIceberg 0.10 /
0.11 does one unparameterized list and returns it). #5533 shows it closer
to home: the generic-tables CLI follows next-page-token only when
--page-size is set, so a configured maximum would drop the rest of the
tables with no error.
I don't think we need a second flag.
Option 1 (cap an explicit pageSize, leave unsized requests complete) is the
original #5282 ceiling, and it is Iceberg-legal: pageSize is an upper
bound. It is not server protection. A client that sends no pageToken is the
one asking for every identifier under the namespace; exempting that request
leaves the unbounded case untouched. Option 2 (force pagination on that
request) does bound it, which is why the review grew in that direction, but
returning a partial 200 is the wrong shape for that bound.
Dmitri's fail-closed rule, is the contract I would want from the existing
setting:
• paginated request: cap to LIST_PAGINATION_MAX_PAGE_SIZE
• unsized request that fits: complete list, null next-page-token
• unsized request that would exceed: reject, not a truncated 200
Default stays -1, so nothing changes for deployments that have not opted
in. LIST_PAGINATION_ENABLED=false remains the escape hatch that ignores
pagination parameters. Operators who need a ceiling turn it on and get a
bound they can state and diagnose. Clients that believe they hold a
complete list either do, or they get an error telling them to paginate or
raise the limit.
Two implementation details I think the follow-up PR has to get right, or
reject will not do what we are claiming.
1. Reject has to stop at max+1, not list everything and then throw.
Page.mapped already stops after pageSize items when pagination is
requested, and the JDBC listing path has LIMIT. If the unsized path still
materializes the full result into a List, copies it into the response, and
serializes it, we pay the heap and concurrency cost Ayush described and
then fail. That protects the client from a lie; it does not protect the
server. Probe with size max+1; if a continuation would have been produced,
return 400 with the configured maximum in the message. The federated path
needs the same rule. Today, when the maximum is set and pageToken is null,
IcebergCatalogHandler injects INITIAL_PAGE_TOKEN so the remote listing is
sliced in memory. That is the same silent truncation. Request at most max+1
from the remote catalog and reject if more remain.
2. Distinguish a missing pageToken from an empty one. The Iceberg REST spec
is explicit: clients start pagination by sending an empty pageToken; if the
parameter is not set, the server must return the complete result. Java
Iceberg always sends pageToken= and follows next-page-token even when
rest-page-size is unset. PageTokenUtil currently treats null and "" the
same, which is harmless for force-pagination and harmful for reject: a
naive "unsized means reject if over max" would 400 the Java client that was
already prepared to page. Reject only when pageToken is absent (and
pageSize is absent). Empty pageToken, or an explicit pageSize, is
pagination and should be capped.
On the overload question: Yufei's uncompressed JSON sizes are right for the
wire. I have not seen a published listTables OOM either. I still think a
default-off ceiling is a reasonable operator tool, peak heap is several
in-memory copies, held for the duration of the response, multiplied by
concurrency, and federation currently pulls the remote list into Polaris
before slicing it. We don't need a production incident to keep the knob. We
do need the knob not to return a 200 that looks complete and is not.
Thanks,
Prithvi S
On Sat, Sep 19, 2026 at 7:02 AM Ayush Saxena <ay...@apache.org> wrote:
> Hi Yufei, Dmitri,
> Thanks both for working through this.
>
> I'm inclined towards Dmitri's suggestion of throwing an exception. It
> gives a contract that is easy to state and easy to diagnose: when a
> maximum is configured, that value is the bound the server will honour.
> A paginated request is capped to it; an unpaged request that fits still
> gets the complete list; an unpaged request that would exceed it is
> rejected rather than quietly truncated. A client that believes it holds
> a complete list when it does not is the failure worth designing against.
> If that sounds reasonable, I can raise a PR for it.
>
> On the overload question, I don't think the per-response payload is the
> whole picture. For a request without pagination the full result set is
> materialized in heap before anything is written — Page.mapped collects
> every row into a list, the handler copies it into the response
> collection, and it is then serialized — so peak memory is a multiple of
> the figures in the table, and it is held for the duration of the
> response rather than streamed. That cost also multiplies with
> concurrency rather than with a single listing, and a large response
> occupies the connection while it is written, so a handful of
> simultaneous unbounded lists is a different proposition from one. A
> configurable ceiling is what lets an operator put a bound on that.
>
> For context on where the flag came from: it started with a review
> comment [1].
>
> -Ayush
> [1] https://github.com/apache/polaris/pull/5255#discussion_r3739619320
>
> On 2026/09/19 00:02:17 Dmitri Bourlatchkov wrote:
> > 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 <fl...@gmail.com> 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 <di...@apache.org>
> > > 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 <ayushsaxena@apache.org
> >
> > > > 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
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Separate list page-size limits from forced pagination
Posted by Ayush Saxena <ay...@apache.org>.
Hi Yufei, Dmitri,
Thanks both for working through this.
I'm inclined towards Dmitri's suggestion of throwing an exception. It
gives a contract that is easy to state and easy to diagnose: when a
maximum is configured, that value is the bound the server will honour.
A paginated request is capped to it; an unpaged request that fits still
gets the complete list; an unpaged request that would exceed it is
rejected rather than quietly truncated. A client that believes it holds
a complete list when it does not is the failure worth designing against.
If that sounds reasonable, I can raise a PR for it.
On the overload question, I don't think the per-response payload is the
whole picture. For a request without pagination the full result set is
materialized in heap before anything is written — Page.mapped collects
every row into a list, the handler copies it into the response
collection, and it is then serialized — so peak memory is a multiple of
the figures in the table, and it is held for the duration of the
response rather than streamed. That cost also multiplies with
concurrency rather than with a single listing, and a large response
occupies the connection while it is written, so a handful of
simultaneous unbounded lists is a different proposition from one. A
configurable ceiling is what lets an operator put a bound on that.
For context on where the flag came from: it started with a review
comment [1].
-Ayush
[1] https://github.com/apache/polaris/pull/5255#discussion_r3739619320
On 2026/09/19 00:02:17 Dmitri Bourlatchkov wrote:
> 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 <fl...@gmail.com> 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 <di...@apache.org>
> > 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 <ay...@apache.org>
> > > 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
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Separate list page-size limits from forced pagination
Posted by Dmitri Bourlatchkov <di...@apache.org>.
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 <fl...@gmail.com> 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 <di...@apache.org>
> 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 <ay...@apache.org>
> > 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
> > > >
> > >
> >
>
Re: [DISCUSS] Separate list page-size limits from forced pagination
Posted by Yufei Gu <fl...@gmail.com>.
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 <di...@apache.org>
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 <ay...@apache.org>
> 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
> > >
> >
>
Re: [DISCUSS] Separate list page-size limits from forced pagination
Posted by Dmitri Bourlatchkov <di...@apache.org>.
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 <ay...@apache.org> 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
> >
>
Re: [DISCUSS] Separate list page-size limits from forced pagination
Posted by Ayush Saxena <ay...@apache.org>.
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
>