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

New cid summary api and simplified cid info #730

Merged
merged 11 commits into from
Jan 18, 2021

Conversation

asutula
Copy link
Member

@asutula asutula commented Dec 16, 2020

The existing Data.CidInfo API is just too much data, different types of data, in a list, and it gets too big to consume efficiently. This creates a "summary" api that will return the most basic information about all the cids you have stored in powergate, then you can call Data.CidInfo with any one of those cids to still get detailed and varied information about a single cid.

Ended up merging a couple other PRs into this one. For completeness, the are:

@asutula asutula self-assigned this Dec 16, 2020
@asutula asutula requested a review from jsign December 16, 2020 00:27
// CidInfo returns information about cids managed by the FFS instance.
func (s *Service) CidInfo(ctx context.Context, req *userPb.CidInfoRequest) (*userPb.CidInfoResponse, error) {
// CidSummary gives a summary of the storage and jobs state of the specified cid.
func (s *Service) CidSummary(ctx context.Context, req *userPb.CidSummaryRequest) (*userPb.CidSummaryResponse, error) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is surprisingly slow, and I need to figure out why. When I call it against our powergate instance with about 700 cids stored, it takes 10s of seconds to return the data. The amount of data is small, so it must be some processing in here that is taking a while.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure when you test that, but today there were some heavy processes in the Atlas db.
Maybe you got unlucky? Mentioning it since was a special day today.
Dismiss if isn't the case... might be a real prob.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested before the downtime. Will have to dig in and see what's up.

repeated string cids = 1;
}

message CidSummaryResponse {
repeated CidSummary cid_summary = 1;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new request/response type.

message CidInfoResponse {
repeated CidInfo cid_infos = 1;
CidInfo cid_info = 1;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CidInfo request/response is now just for a single cid.

bool stored = 2;
repeated string queued_jobs = 3;
string executing_job = 4;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary returns some simple inflation about a cid.

Copy link
Contributor

@jsign jsign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

// CidInfo returns information about cids managed by the FFS instance.
func (s *Service) CidInfo(ctx context.Context, req *userPb.CidInfoRequest) (*userPb.CidInfoResponse, error) {
// CidSummary gives a summary of the storage and jobs state of the specified cid.
func (s *Service) CidSummary(ctx context.Context, req *userPb.CidSummaryRequest) (*userPb.CidSummaryResponse, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure when you test that, but today there were some heavy processes in the Atlas db.
Maybe you got unlucky? Mentioning it since was a special day today.
Dismiss if isn't the case... might be a real prob.

api/server/user/data.go Outdated Show resolved Hide resolved
api/server/user/data.go Show resolved Hide resolved
api/server/user/data.go Outdated Show resolved Hide resolved
api/server/user/util.go Outdated Show resolved Hide resolved
cmd/pow/cmd/data_summary.go Outdated Show resolved Hide resolved
@asutula asutula force-pushed the asutula/cid-info-summary-split branch from 3965f92 to 067c664 Compare January 16, 2021 03:56
@asutula asutula changed the base branch from master to develop January 16, 2021 03:57
@asutula
Copy link
Member Author

asutula commented Jan 16, 2021

Ok all rebased against develop here and changed the target of the PR to develop.

Signed-off-by: Aaron Sutula <[email protected]>
* move data cli commands to packages, refactor common code

Signed-off-by: Aaron Sutula <[email protected]>

* add data summary command that was forgotten

Signed-off-by: Aaron Sutula <[email protected]>

* apply cli org to the rest

Signed-off-by: Aaron Sutula <[email protected]>

* update docs

Signed-off-by: Aaron Sutula <[email protected]>

* Data summary TUI (#729)

* basic tui for pow data summary

Signed-off-by: Aaron Sutula <[email protected]>

* pr feedback

Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
* first cut a a list implementation for storage jobs

Signed-off-by: Aaron Sutula <[email protected]>

* bubble up list storage jobs api through scheduler, ffs, and apis

Signed-off-by: Aaron Sutula <[email protected]>

* client for list, better name for next page token

Signed-off-by: Aaron Sutula <[email protected]>

* list cli

Signed-off-by: Aaron Sutula <[email protected]>

* fix selector filter

Signed-off-by: Aaron Sutula <[email protected]>

* remove old/redundant code

Signed-off-by: Aaron Sutula <[email protected]>

* better alias

Signed-off-by: Aaron Sutula <[email protected]>

* pr feedback

Signed-off-by: Aaron Sutula <[email protected]>

* remove counts

Signed-off-by: Aaron Sutula <[email protected]>

* migration

Signed-off-by: Aaron Sutula <[email protected]>

* good tests for sjstore.List

Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
@asutula asutula force-pushed the asutula/cid-info-summary-split branch from 1b973e7 to 86358b5 Compare January 16, 2021 20:05
Signed-off-by: Ignacio Hagopian <[email protected]>
@asutula asutula merged commit 71ed460 into develop Jan 18, 2021
@asutula asutula deleted the asutula/cid-info-summary-split branch January 18, 2021 19:07
jsign added a commit that referenced this pull request Jan 21, 2021
* new summary api and simplified cid info

Signed-off-by: Aaron Sutula <[email protected]>

* pr feedback

Signed-off-by: Aaron Sutula <[email protected]>

* Proposed CLI package structure (#728)

* move data cli commands to packages, refactor common code

Signed-off-by: Aaron Sutula <[email protected]>

* add data summary command that was forgotten

Signed-off-by: Aaron Sutula <[email protected]>

* apply cli org to the rest

Signed-off-by: Aaron Sutula <[email protected]>

* update docs

Signed-off-by: Aaron Sutula <[email protected]>

* Data summary TUI (#729)

* basic tui for pow data summary

Signed-off-by: Aaron Sutula <[email protected]>

* pr feedback

Signed-off-by: Aaron Sutula <[email protected]>

* update docs

Signed-off-by: Aaron Sutula <[email protected]>

* List storage jobs query API (#748)

* first cut a a list implementation for storage jobs

Signed-off-by: Aaron Sutula <[email protected]>

* bubble up list storage jobs api through scheduler, ffs, and apis

Signed-off-by: Aaron Sutula <[email protected]>

* client for list, better name for next page token

Signed-off-by: Aaron Sutula <[email protected]>

* list cli

Signed-off-by: Aaron Sutula <[email protected]>

* fix selector filter

Signed-off-by: Aaron Sutula <[email protected]>

* remove old/redundant code

Signed-off-by: Aaron Sutula <[email protected]>

* better alias

Signed-off-by: Aaron Sutula <[email protected]>

* pr feedback

Signed-off-by: Aaron Sutula <[email protected]>

* remove counts

Signed-off-by: Aaron Sutula <[email protected]>

* migration

Signed-off-by: Aaron Sutula <[email protected]>

* good tests for sjstore.List

Signed-off-by: Aaron Sutula <[email protected]>

* update pow docs

Signed-off-by: Aaron Sutula <[email protected]>

* fix compilation problems

Signed-off-by: Aaron Sutula <[email protected]>

* update docs

Signed-off-by: Aaron Sutula <[email protected]>

* comment exported func

Signed-off-by: Aaron Sutula <[email protected]>

* fix docs

Signed-off-by: Ignacio Hagopian <[email protected]>

* add missin cmd to cli, better name for admin jobs cli

Signed-off-by: Aaron Sutula <[email protected]>

Co-authored-by: Ignacio Hagopian <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants