Skip to content
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

Bundling/streamlining requests for indices page. #2017

Merged
merged 16 commits into from Apr 11, 2016
Merged

Bundling/streamlining requests for indices page. #2017

merged 16 commits into from Apr 11, 2016

Conversation

@dennisoelkers
Copy link
Member

@dennisoelkers dennisoelkers commented Apr 4, 2016

  • Add endpoints for indices overview and multiple index details.
  • Add new store/actions which make use of new index/indices overview.
  • Allows components to (un-)/register for index details.
  • Use new index subscriptions and index/indices overview in components.

Fixes #1959, #1960

@dennisoelkers dennisoelkers added the web label Apr 4, 2016
@dennisoelkers dennisoelkers added this to the 2.0.0 milestone Apr 4, 2016
@edmundoa edmundoa self-assigned this Apr 4, 2016

@GET
@Timed
@ApiOperation(value = "Get overview of current indexing state, including deflector config, cluster state, index rnages & message counts.")

This comment has been minimized.

@edmundoa

edmundoa Apr 4, 2016
Member

Ranges is misspelled :)

This comment has been minimized.

@dennisoelkers

dennisoelkers Apr 4, 2016
Author Member

Oh, it's not spelled rnages? :)

return { indices: this.indices, closedIndices: this.closedIndices };
});

IndicesActions.list.promise(promise);

This comment has been minimized.

@edmundoa

edmundoa Apr 4, 2016
Member

Shouldn't this method use the multiple action to store the promise?

This comment has been minimized.

@dennisoelkers

dennisoelkers Apr 4, 2016
Author Member

👍

const url = URLUtils.qualifyUrl(ApiRoutes.IndexerOverviewApiResource.list().url);
const promise = fetch('GET', url);
promise.then((response) => {
this.trigger(response);

This comment has been minimized.

@edmundoa

edmundoa Apr 4, 2016
Member

I don't really like this, as the response is returning several keys that will go into a component's state (at least if it's connecting to the store), and there's no obvious way of knowing which keys they are unless you look into the response. Can we make the keys explicit here?

This comment has been minimized.

@dennisoelkers

dennisoelkers Apr 4, 2016
Author Member

Agreed!

@@ -8,21 +8,25 @@ const IndicesOverview = React.createClass({
propTypes: {
closedIndices: React.PropTypes.array.isRequired,
deflector: React.PropTypes.object.isRequired,
indexRanges: React.PropTypes.array.isRequired,
indexDetails: React.PropTypes.object.isRequired,
indices: React.PropTypes.object.isRequired,
},
_isDeflector(index) {

This comment has been minimized.

@edmundoa

edmundoa Apr 4, 2016
Member

This function is unused now, we can remove it.

const deleted = this.props.index.all_shards.documents.deleted;
const sizes = this.props.index.size;
if (sizes) {
const count = sizes.count;

This comment has been minimized.

@edmundoa

edmundoa Apr 4, 2016
Member

Is this calculation right? It looks like count is not part of IndexSizeSummary.

labels.push(<Label key={`${this.props.name}-closed-label`} bsStyle="warning">closed</Label>);
}

if (index.is_reopened) {

This comment has been minimized.

@edmundoa

edmundoa Apr 4, 2016
Member

When a reopened index is closed again, we display two labels for it, even though we should only display closed in that case.

This comment has been minimized.

@dennisoelkers

dennisoelkers Apr 4, 2016
Author Member

No, this is not the case, as the backend does not list a reopened index as closed.

This comment has been minimized.

@edmundoa

edmundoa Apr 4, 2016
Member

Probably it shouldn't, but it does:

"graylog2_0": {
  "size":null,
  "range":null,
  "is_closed":true,
  "is_reopened":true,
  "is_deflector":false
}

screen shot 2016-04-04 at 14 39 10

This comment has been minimized.

@dennisoelkers

dennisoelkers Apr 4, 2016
Author Member

True. Filed an issue (#2019) and worked around this in the IndexerOverviewResource.


<small>
{this._formatLabels(index)}{' '}
{this._formatIndexRange(index)}{' '}

<IndexSizeSummary index={index} />

This comment has been minimized.

@edmundoa

edmundoa Apr 4, 2016
Member

IndexSizeSummary is not updated to the new data structure coming from the API, and it's never rendered.

This comment has been minimized.

@dennisoelkers

dennisoelkers Apr 4, 2016
Author Member

👍 Fixed in 522f412

@dennisoelkers dennisoelkers force-pushed the issue-1960 branch from 22e13f9 to 5dd52a3 Apr 7, 2016
dennisoelkers added 15 commits Apr 1, 2016
This helps us to keep the required payload for updating the indices page
minimal, because we can get the required information to render the
initial page and the details for selected indices separately.
Right now, closing an index does not clear its reopened flag (#2019).
Therefore we are filtering that out explicitly for closed indices, so no
double badges (which would be misleading) are displayed.
This is prefetching the reopened status for existing indices in the
IndexerOverviewResource and the IndicesResource for bulk listings of
reopened indices.
@dennisoelkers dennisoelkers force-pushed the issue-1960 branch from 5dd52a3 to 832d877 Apr 7, 2016
@edmundoa edmundoa merged commit 6c4021e into master Apr 11, 2016
4 checks passed
4 checks passed
ci-server-integration Jenkins build graylog2-server-integration-pr 802 has succeeded
Details
ci-web-linter Jenkins build graylog-pr-linter-check 291 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@edmundoa edmundoa deleted the issue-1960 branch Apr 11, 2016
@edmundoa
Copy link
Member

@edmundoa edmundoa commented Apr 11, 2016

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.