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

Feat/search load #91

Merged
merged 2 commits into from
Aug 12, 2024
Merged

Feat/search load #91

merged 2 commits into from
Aug 12, 2024

Conversation

carpawell
Copy link
Member

No description provided.

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

Attention: Patch coverage is 0% with 33 lines in your changes missing coverage. Please review.

Project coverage is 2.11%. Comparing base (9254250) to head (abecf3c).
Report is 1 commits behind head on master.

Files Patch % Lines
internal/native/client.go 0.00% 32 Missing ⚠️
internal/native/native.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           master     #91      +/-   ##
=========================================
- Coverage    2.20%   2.11%   -0.10%     
=========================================
  Files          11      11              
  Lines         725     758      +33     
=========================================
  Hits           16      16              
- Misses        709     742      +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

internal/native/client.go Show resolved Hide resolved
internal/native/client.go Show resolved Hide resolved
@carpawell carpawell requested a review from smallhive July 25, 2024 13:02
@EESergey
Copy link
Contributor

The search time should be calculated as the total search time in the container divided by the number of objects; it should not depend on the number of objects found.

@EESergey
Copy link
Contributor

@cthulhu-rider Are you interested in the scenario when you specify a json file instead of a cid?

smallhive
smallhive previously approved these changes Jul 26, 2024
Value string
}

func (c *Client) Search(cnrString string, filtersJS []filter) (int, error) {

Choose a reason for hiding this comment

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

lets add docs at least to know what does int return means

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 doc is not expected to be read by anyone. this code is also impossible to use by a human, only as an extension inside a virtual machine

there are some well-known extensions that are "documented" via examples: https://github.com/grafana/xk6-loki/tree/5248b47b4efdd234403ba13e6f46dc173f9a142e/examples or via a readmi file (https://github.com/grafana/xk6-loki/blob/5248b47b4efdd234403ba13e6f46dc173f9a142e/README.md)

not sure if it applies to us (#85)

Choose a reason for hiding this comment

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

is this an autogen code? If not, any dev can read and support it. Small tip would be appreciated

Copy link
Member Author

Choose a reason for hiding this comment

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

it is not a problem to me, added a comment. now we have one documented method out of 7

Value string
}

func (c *Client) Search(cnrString string, filtersJS []filter) (int, error) {

Choose a reason for hiding this comment

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

why filter isnt exported?

Copy link
Member Author

@carpawell carpawell Aug 6, 2024

Choose a reason for hiding this comment

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

why it should be? it is only a container that will be used when code is translated from JS to Go

Copy link

@cthulhu-rider cthulhu-rider Aug 9, 2024

Choose a reason for hiding this comment

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

why method is exported then? And the fields. im lookin at this code as a regular Go one

Copy link
Member Author

Choose a reason for hiding this comment

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

why method is exported then?

to make it possible to be called in k6 code

im lookin at this code as a regular Go one

idk, i think this code should be considered as a private package code, it is called by an external k6 code that is never human (and that is never written based on our neofs code) and that just looks for exported fields with appropriate names. can be exported, OK, but changes nothing

scenarios/grpc_search.js Show resolved Hide resolved
import { check } from 'k6';
import native from 'k6/x/neofs/native';

const grpc_client = native.connect(__ENV.GRPC_ENDPOINT, '', 5, 15);

Choose a reason for hiding this comment

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

id make consts for non-obvious literals

Copy link
Member Author

Choose a reason for hiding this comment

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

dont mind

Choose a reason for hiding this comment

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

and empty string pls, i can only imagine what does it mean

@cthulhu-rider
Copy link

@cthulhu-rider Are you interested in the scenario when you specify a json file instead of a cid?

me not, current approach looks similar to neofs-cli

EESergey
EESergey previously approved these changes Jul 29, 2024
@carpawell carpawell dismissed stale reviews from EESergey and smallhive via 707028f August 6, 2024 17:46
@carpawell carpawell force-pushed the feat/search-load branch 2 times, most recently from 707028f to 81c72a2 Compare August 6, 2024 17:47
@carpawell carpawell requested a review from cthulhu-rider August 6, 2024 17:50
smallhive
smallhive previously approved these changes Aug 9, 2024
Simple operation, supports generic filters and relative metrics (seconds per
single object in results).

Signed-off-by: Pavel Karpy <[email protected]>
It does what it claims, however, it may be adjusted to real requirements in the
future. Controlled by a number of iterations and number of virtual users.
Searches for objects with `test=test` attributes. Closes #90.

Run example:
```
▶ ./xk6-neofs run -e i=200 -e vu=1 -e cid=FB6UeNbx65iHuNKNbGsb2xHxooU7kXangaUxv9P6h9uW -e GRPC_ENDPOINT=s01.neofs.devenv:8080 scenarios/grpc_search.js
```

Signed-off-by: Pavel Karpy <[email protected]>
@roman-khimov roman-khimov merged commit bcbeb1c into master Aug 12, 2024
11 of 13 checks passed
@roman-khimov roman-khimov deleted the feat/search-load branch August 12, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants