Skip to content

/graylog2-server

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pagination for views list always shows only single page #6254

Closed
dennisoelkers opened this issue Aug 7, 2019 · 0 comments · Fixed by #6256

Comments

@dennisoelkers
Copy link
Member

commented Aug 7, 2019

Expected Behavior

When more than 10 views exist, the views list should show multiple pages in the pagination.

Current Behavior

When more than 10 views exist, only a single page is shown.

Possible Solution

Steps to Reproduce (for bugs)

  1. Create more than 10 views
  2. Go to views list
  3. Try to access view no. 11

BrokenViewsListPagination

Context

Your Environment

  • Graylog Version: 3.1.0-rc.2-SNAPSHOT
  • Elasticsearch Version:
  • MongoDB Version:
  • Operating System:
  • Browser version:

@dennisoelkers dennisoelkers added this to the 3.1.0 milestone Aug 7, 2019

dennisoelkers added a commit that referenced this issue Aug 7, 2019
@dennisoelkers
Avoid using `Stream#peek` for counting items in paginated result set. 
Before this change, in
`PaginatedDbService#findPaginatedWithQueryFilterAndSort` a single stream
was used to filter (based on the query), paginate (based on page size
and page number) and determine the number of total items (before
pagination). The latter is performed using `Stream#peek`, which has the
following characteristics:

  1. The docs say: `This method exists mainly to support debugging, where
you want to see the elements as they flow past a certain point in a
pipeline`
  2. `peek` is an intermediate operation on a stream, which means that
    a) it's actual execution is driven by the terminal operation of the stream
    b) it has the benefit in this case that a single stream execution is
able to perform multiple tasks

The code before this PR relies on `peek` being executed for each item
on the stream before pagination (`skip`/`limit`) is performed. This is
the case for OracleJDK up and including 1.8.0_202.

Unfortunately OpenJDK is performing an optimization, where the actual
stream execution is determined by the terminal operation (see 2a) and
`peek` only sees items after filtering _and_ pagination.

Therefore, this change is replacing the usage of `peek` with a simpler
variant, which is using two separate stream instantiations for counting
the total items and the actual pagination.

Fixes #6254.
Verified
This commit was signed with a verified signature.
dennisoelkers Dennis Oelkers
GPG key ID: 440E58BD30A068C8 Learn about signing commits
Loading status checks…
7de99a2
@dennisoelkers dennisoelkers referenced this issue Aug 7, 2019
Merged
4 of 9 tasks complete

@bernd bernd closed this in #6256 Aug 7, 2019

bernd added a commit that referenced this issue Aug 7, 2019
@dennisoelkers @bernd
Avoid using `Stream#peek` for counting items in paginated result set. (… 
…#6256)

* Avoid using `Stream#peek` for counting items in paginated result set.

Before this change, in
`PaginatedDbService#findPaginatedWithQueryFilterAndSort` a single stream
was used to filter (based on the query), paginate (based on page size
and page number) and determine the number of total items (before
pagination). The latter is performed using `Stream#peek`, which has the
following characteristics:

  1. The docs say: `This method exists mainly to support debugging, where
you want to see the elements as they flow past a certain point in a
pipeline`
  2. `peek` is an intermediate operation on a stream, which means that
    a) it's actual execution is driven by the terminal operation of the stream
    b) it has the benefit in this case that a single stream execution is
able to perform multiple tasks

The code before this PR relies on `peek` being executed for each item
on the stream before pagination (`skip`/`limit`) is performed. This is
the case for OracleJDK up and including 1.8.0_202.

Unfortunately OpenJDK is performing an optimization, where the actual
stream execution is determined by the terminal operation (see 2a) and
`peek` only sees items after filtering _and_ pagination.

Therefore, this change is replacing the usage of `peek` with a simpler
variant, which is using two separate stream instantiations for counting
the total items and the actual pagination.

Fixes #6254.

* Updated comments.

* Ensuring closing of streams by using try-with-resource.
70dc8e7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
  3.1.0
1 participant
@dennisoelkers
You can’t perform that action at this time.