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

implement load tester prototype #363

Merged
merged 28 commits into from
Jun 5, 2020
Merged

implement load tester prototype #363

merged 28 commits into from
Jun 5, 2020

Conversation

kmrmt
Copy link
Contributor

@kmrmt kmrmt commented May 1, 2020

Description:

I added load test code for vald with internal/client and pkg package structure

Related Issue:

#346

How Has This Been Tested?:

This PR is Work in Progress.

Environment:

  • Golang Version: 1.14
  • Docker Version: 19.03.5
  • Kubernetes Version: 1.17.3
  • NGT Version: 1.9.1

Types of changes:

  • Bug fix [type/bug]
  • New feature [type/feature]
  • Add tests [type/test]
  • Security related changes [type/security]
  • Add documents [type/documentation]
  • Refactoring [type/refactoring]
  • Update dependencies [type/dependency]
  • Update benchmarks and performances [type/bench]
  • Update CI [type/ci]

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Checklist:

  • I have read the CONTRIBUTING document.
  • I have checked open Pull Requests for the similar feature or fixes?
  • I have added tests and benchmarks to cover my changes.
  • I have ensured all new and existing tests passed.
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly.

@pull-assistant
Copy link

pull-assistant bot commented May 1, 2020

Score: 0.90

Best reviewed: commit by commit


Optimal code review plan (10 warnings, 2 commits squashed)

[WIP]implement load tester prototype

.../cli/loadtest/config/config.go 50% changes removed in 🤖 Update licen...

...dtest/service/search/search.go 57% changes removed in refactor

...ervice/search/search_option.go 61% changes removed in refactor

...s/cli/loadtest/service/load.go 96% changes removed in refactor

...s/cli/loadtest/usecase/load.go 54% changes removed in fix as per the comme...

...dtest/service/insert/insert.go 53% changes removed in fix: refactor

...ervice/insert/insert_option.go 61% changes removed in refactor

use errgroup

.../cli/loadtest/config/config.go 50% changes removed in add configuration

...ervice/insert/insert_option.go 67% changes removed in use internal/net/grp...

...s/cli/loadtest/usecase/load.go 43% changes removed in fix based on the com...

...dtest/service/search/search.go 47% changes removed in fix

...ervice/search/search_option.go 67% changes removed in fix search

...dtest/service/insert/insert.go 53% changes removed in refactor

add variable length id generator

...dtest/service/search/search.go 50% changes removed in refactor

fix based on the comments

...s/cli/loadtest/usecase/load.go 48% changes removed in fix: refactor

.../loadtest/usecase/operation.go 41% changes removed in fix: refactor

fix

...dtest/service/insert/insert.go 71% changes removed in refactor

     add configuration

     use internal/net/grpc

use errgroup

cmd/cli/loadtest/sample.yaml 50% changes removed in fix

...dtest/service/insert/insert.go 60% changes removed in fix: apply suggestio...

     fix search

     fix

fix

...s/cli/loadtest/usecase/load.go 50% changes removed in fix: refactor

     fix

refactor

...cli/loadtest/service/insert.go 41% changes removed in fix: refactor

.../cli/loadtest/config/config.go 67% changes removed in 🤖 Update licen...

     add logging

     bug fix

🤖 Update license headers / Format go codes and yaml files

.../loadtest/usecase/operation.go 56% changes removed in fix: refactor

     fix as per the comment

add comments ... add comments

Squashed 2 commits:

...loadtest/assets/loader_test.go 50% changes removed in fix: make gotests/g...

     modify go modules

     fix file permission

     fix: apply suggestion

     fix: option setings

     fix: apply suggestion

     fix: refactor

     Merge branch 'master' into feature/benchmark/load_test

     fix: make gotests/gen

     fix: lint error

Powered by Pull Assistant. Last update c393cd4 ... a00c4ea. Read the comment docs.

@codecov
Copy link

codecov bot commented May 1, 2020

Codecov Report

Merging #363 into master will increase coverage by 0.13%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #363      +/-   ##
=========================================
+ Coverage    7.92%   8.06%   +0.13%     
=========================================
  Files         384     381       -3     
  Lines       19707   19376     -331     
=========================================
  Hits         1562    1562              
+ Misses      17924   17593     -331     
  Partials      221     221              
Impacted Files Coverage Δ
internal/net/grpc/client.go 0.00% <0.00%> (ø)
pkg/tools/cli/loadtest/config/config.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4707d8...a00c4ea. Read the comment docs.

@kmrmt kmrmt requested a review from hlts2 May 14, 2020 01:11
hack/benchmark/internal/assets/dataset.go Show resolved Hide resolved
pkg/tools/cli/loadtest/service/search/search.go Outdated Show resolved Hide resolved
pkg/tools/cli/loadtest/usecase/load.go Outdated Show resolved Hide resolved
pkg/tools/cli/loadtest/usecase/load.go Outdated Show resolved Hide resolved
pkg/tools/cli/loadtest/usecase/load.go Outdated Show resolved Hide resolved
pkg/tools/cli/loadtest/service/insert/insert.go Outdated Show resolved Hide resolved
pkg/tools/cli/loadtest/service/insert/insert.go Outdated Show resolved Hide resolved
pkg/tools/cli/loadtest/service/search/search.go Outdated Show resolved Hide resolved
@kmrmt kmrmt marked this pull request as ready for review May 18, 2020 23:54
@kmrmt kmrmt changed the title [WIP]implement load tester prototype implement load tester prototype May 18, 2020
@kevindiu
Copy link
Contributor

Can you tell me how to execute the load test? I executed go run cmd/cli/loadtest/main.go -c cmd/cli/loadtest/sample.yaml but seems it doesn't work..

% go run cmd/cli/loadtest/main.go -c cmd/cli/loadtest/sample.yaml
{"Date":"2020-05-19T13:54:25.209302+09:00","Level":"INFO","Detail":"maxprocs: Leaving GOMAXPROCS=4: CPU quota undefined"}
{"Date":"2020-05-19T13:54:25.209302+09:00","Level":"INFO","Detail":"service load test v0.0.0 starting..."}
{"Date":"2020-05-19T13:54:25.209302+09:00","Level":"WARN","Detail":"creating new connection pool for addr= 0.0.0.0:8081"}
{"Date":"2020-05-19T13:54:25.209302+09:00","Level":"WARN","Detail":"connecting to new connection pool for addr= 0.0.0.0:8081"}
{"Date":"2020-05-19T13:54:25.209302+09:00","Level":"DEBG","Detail":"establishing same connection to 0.0.0.0:8081"}
{"Date":"2020-05-19T13:54:25.209302+09:00","Level":"DEBG","Detail":"dialing to 0.0.0.0:8081"}
{"Date":"2020-05-19T13:54:26.209645+09:00","Level":"DEBG","Detail":"failed to dial pool connection addr = 0.0.0.0:8081\terror = context deadline exceeded"}
{"Date":"2020-05-19T13:54:26.209645+09:00","Level":"FATAL","Detail":[{}]}
exit status 1

@kevindiu
Copy link
Contributor

the following error occurs if I run the load test main.go. After I remove that line it works.

% go run main.go
# github.com/vdaas/vald/pkg/tools/cli/loadtest/service/insert
../../../pkg/tools/cli/loadtest/service/insert/insert.go:21:2: imported and not used: "github.com/vdaas/vald/internal/log"

@kmrmt kmrmt force-pushed the feature/benchmark/load_test branch from ae7899a to 6e0fa11 Compare May 25, 2020 05:22
@kmrmt
Copy link
Contributor Author

kmrmt commented May 25, 2020

/rebase

@vdaas-ci
Copy link
Collaborator

[REBASE] Rebase triggered by kmrmt for branch: feature/benchmark/load_test

@vdaas-ci vdaas-ci force-pushed the feature/benchmark/load_test branch from 6e0fa11 to b7c2b09 Compare May 25, 2020 05:25
@kevindiu
Copy link
Contributor

I execute make bench command but seems it is not working...

% make bench
rm -rf pprof/core/ngt
mkdir -p pprof/core/ngt
## starting sequential core benchmark
go test -count=1 -timeout=1h -bench=NGTSequential -benchmem -o pprof/core/ngt/core.bin -cpuprofile pprof/core/ngt/cpu-sequential.out -memprofile pprof/core/ngt/mem-sequential.out -trace pprof/core/ngt/trace-sequential.out  ./hack/benchmark/core/ngt/ngt_bench_test.go -dataset=identity-128 | tee pprof/core/ngt/result-sequential.out
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x44ec4c2]

goroutine 36 [running]:
github.com/vdaas/vald/internal/log.Debugf(...)
	/Users/wdiu/go/src/github.com/vdaas/vald/internal/log/log.go:76
github.com/vdaas/vald/pkg/tools/cli/loadtest/assets.Data(0x7ffeefbffa8a, 0xc, 0x1)
	/Users/wdiu/go/src/github.com/vdaas/vald/pkg/tools/cli/loadtest/assets/dataset.go:340 +0x82
github.com/vdaas/vald/hack/benchmark/internal/assets.Data.func1(0x46e0c80, 0xc00018c000, 0xc00000e180, 0x2)
	/Users/wdiu/go/src/github.com/vdaas/vald/hack/benchmark/internal/assets/dataset.go:29 +0x5f
github.com/vdaas/vald/hack/benchmark/core/benchmark.New(0xc00018c000, 0xc00033df10, 0x2, 0x2, 0xc000182080, 0x5ecb7f17)
	/Users/wdiu/go/src/github.com/vdaas/vald/hack/benchmark/core/benchmark/benchmark.go:49 +0x152
command-line-arguments.BenchmarkNGTSequential_Insert(0xc00018c000)
	/Users/wdiu/go/src/github.com/vdaas/vald/hack/benchmark/core/ngt/ngt_bench_test.go:64 +0x181
testing.(*B).runN(0xc00018c000, 0x1)
	/usr/local/Cellar/go/1.14.3/libexec/src/testing/benchmark.go:191 +0xe8
testing.(*B).run1.func1(0xc00018c000)
	/usr/local/Cellar/go/1.14.3/libexec/src/testing/benchmark.go:231 +0x57
created by testing.(*B).run1
	/usr/local/Cellar/go/1.14.3/libexec/src/testing/benchmark.go:224 +0x7d
exit status 2
FAIL	command-line-arguments	1.971s
FAIL
go tool pprof --svg pprof/core/ngt/core.bin pprof/core/ngt/cpu-sequential.out > pprof/core/ngt/cpu-sequential.svg
pprof/core/ngt/cpu-sequential.out: parsing profile: empty input file
failed to fetch any source profiles
make: *** [bench/core/ngt/sequential] Error 1

@hlts2
Copy link
Contributor

hlts2 commented May 25, 2020

I think the error of internal/errors is better than error of fmt.
For example, the following.
https://github.com/vdaas/vald/pull/363/files#diff-5530c935b48c0cc3fede73dec8301849R61

What do you think?

@kmrmt
Copy link
Contributor Author

kmrmt commented May 25, 2020

Thank you for your review!
Sorry, I don't know about internal/errors package.
I am fixing your review.

@kmrmt kmrmt force-pushed the feature/benchmark/load_test branch from b7c2b09 to bf054ab Compare May 25, 2020 08:54
@kmrmt
Copy link
Contributor Author

kmrmt commented May 27, 2020

/rebase
/format

@vdaas-ci
Copy link
Collaborator

[REBASE] Rebase triggered by kmrmt for branch: feature/benchmark/load_test

@vdaas-ci
Copy link
Collaborator

[REBASE] Failed to rebase.

kmrmt and others added 12 commits June 4, 2020 05:31
Signed-off-by: Kosuke Morimoto <kou.morimoto@gmail.com>
Signed-off-by: Kosuke Morimoto <kou.morimoto@gmail.com>
Signed-off-by: Kosuke Morimoto <kou.morimoto@gmail.com>
Signed-off-by: Kosuke Morimoto <kou.morimoto@gmail.com>
Signed-off-by: Kosuke Morimoto <kou.morimoto@gmail.com>
Signed-off-by: Kosuke Morimoto <kou.morimoto@gmail.com>
Signed-off-by: Kosuke Morimoto <kou.morimoto@gmail.com>
@vdaas-ci vdaas-ci dismissed stale reviews from vankichi, hlts2, and kevindiu via 08389e2 June 4, 2020 05:31
@vdaas-ci vdaas-ci force-pushed the feature/benchmark/load_test branch from 13c0989 to 08389e2 Compare June 4, 2020 05:31
hlts2 added 4 commits June 4, 2020 16:06
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
kpango
kpango previously approved these changes Jun 5, 2020
Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

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

LGTM

Yusuke Kato and others added 2 commits June 5, 2020 12:01
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
}

var (
data = map[string]func() (Dataset, error){
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
data is a global variable (gochecknoglobals)

// CreateRandomIDsWithLength generates random string IDs that have specified length.
func CreateRandomIDsWithLength(n, l int) (ids []string) {
ids = make([]string, 0, n)
for i := 0; i < n; i++ {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
for statements should only be cuddled with assignments used in the iteration (wsl)

ObjectType() string
}

type dataset struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
struct of size 296 bytes could be of size 288 bytes (maligned)

}

func Test_dataset_Train(t *testing.T) {
type fields struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
struct of size 296 bytes could be of size 288 bytes (maligned)

}

func Test_dataset_TrainAsFloat64(t *testing.T) {
type fields struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
struct of size 296 bytes could be of size 288 bytes (maligned)

trainOnce: test.fields.trainOnce,
query: test.fields.query,
queryAsFloat64: test.fields.queryAsFloat64,
queryOnce: test.fields.queryOnce,
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
copylocks: literal copies lock value from test.fields.queryOnce: sync.Once contains sync.Mutex (govet)

queryOnce: test.fields.queryOnce,
distances: test.fields.distances,
distancesAsFloat64: test.fields.distancesAsFloat64,
distancesOnce: test.fields.distancesOnce,
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
copylocks: literal copies lock value from test.fields.distancesOnce: sync.Once contains sync.Mutex (govet)


func identity(dim int) func() (Dataset, error) {
return func() (Dataset, error) {
ids := CreateSerialIDs(dim * 1000)
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
mnd: Magic number: 1000, in detected (gomnd)

}

// Do operates load test.
func (l *loader) Do(ctx context.Context) <-chan error {
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
Function 'Do' is too long (78 > 60) (funlen)

var pgCnt int32 = 0
var start time.Time
progress := func() {
log.Infof("progress %d items, %f[qps]", pgCnt, float64(pgCnt)/time.Now().Sub(start).Seconds())
Copy link
Contributor

Choose a reason for hiding this comment

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

[golangci] reported by reviewdog 🐶
S1012: should use time.Since instead of time.Now().Sub (gosimple)

@kpango kpango merged commit 37f290e into master Jun 5, 2020
@kpango kpango deleted the feature/benchmark/load_test branch June 5, 2020 03:48
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.

7 participants