You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@beam.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2020/11/03 13:34:00 UTC
[jira] [Work logged] (BEAM-11075) Load Tests for Go SDK
[ https://issues.apache.org/jira/browse/BEAM-11075?focusedWorklogId=506671&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-506671 ]
ASF GitHub Bot logged work on BEAM-11075:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 03/Nov/20 13:33
Start Date: 03/Nov/20 13:33
Worklog Time Spent: 10m
Work Description: kamilwu commented on a change in pull request #13245:
URL: https://github.com/apache/beam/pull/13245#discussion_r516081963
##########
File path: sdks/go/pkg/beam/io/synthetic/source.go
##########
@@ -165,10 +175,12 @@ type SourceConfigBuilder struct {
func DefaultSourceConfig() *SourceConfigBuilder {
return &SourceConfigBuilder{
cfg: SourceConfig{
- NumElements: 1, // 0 is invalid (drops elements).
- InitialSplits: 1, // 0 is invalid (drops elements).
- KeySize: 8, // 0 is invalid (drops elements).
- ValueSize: 8, // 0 is invalid (drops elements).
+ NumElements: 1, // 0 is invalid (drops elements).
+ InitialSplits: 1, // 0 is invalid (drops elements).
+ KeySize: 8, // 0 is invalid (drops elements).
+ ValueSize: 8, // 0 is invalid (drops elements).
+ HotKeys: 2, // 0 is invalid (drops elements).
Review comment:
Why "2"?
##########
File path: sdks/go/pkg/beam/io/synthetic/source.go
##########
@@ -246,7 +264,8 @@ func (b *SourceConfigBuilder) Build() SourceConfig {
// {
// "num_records": 5,
// "key_size": 5,
-// "value_size": 5
+// "value_size": 5,
+// "num_keys": 5,
Review comment:
`num_keys` -> `num_hot_keys`
##########
File path: sdks/go/pkg/beam/io/synthetic/source.go
##########
@@ -262,8 +281,10 @@ func (b *SourceConfigBuilder) BuildFromJSON(jsonData []byte) SourceConfig {
// synthetic source. It should be created via a SourceConfigBuilder, not by
// directly initializing it (the fields are public to allow encoding).
type SourceConfig struct {
- NumElements int `json:"num_records"`
- InitialSplits int `json:"initial_splits"`
- KeySize int `json:"key_size"`
- ValueSize int `json:"value_size"`
+ NumElements int `json:"num_records"`
+ InitialSplits int `json:"initial_splits"`
+ HotKeys int `json:"hot_keys"`
Review comment:
Python tests use `num_hot_keys`, let's do the same in Go. `hot_key_fraction` is correct.
##########
File path: sdks/go/pkg/beam/io/synthetic/source.go
##########
@@ -165,10 +175,12 @@ type SourceConfigBuilder struct {
func DefaultSourceConfig() *SourceConfigBuilder {
return &SourceConfigBuilder{
cfg: SourceConfig{
- NumElements: 1, // 0 is invalid (drops elements).
- InitialSplits: 1, // 0 is invalid (drops elements).
- KeySize: 8, // 0 is invalid (drops elements).
- ValueSize: 8, // 0 is invalid (drops elements).
+ NumElements: 1, // 0 is invalid (drops elements).
+ InitialSplits: 1, // 0 is invalid (drops elements).
+ KeySize: 8, // 0 is invalid (drops elements).
+ ValueSize: 8, // 0 is invalid (drops elements).
+ HotKeys: 2, // 0 is invalid (drops elements).
Review comment:
By default, there should be no hot keys at all (every key should be unique).
##########
File path: sdks/go/pkg/beam/io/synthetic/source.go
##########
@@ -235,6 +247,12 @@ func (b *SourceConfigBuilder) Build() SourceConfig {
if b.cfg.ValueSize <= 0 {
panic(fmt.Sprintf("SourceConfig.ValueSize must be >= 1. Got: %v", b.cfg.ValueSize))
}
+ if b.cfg.HotKeys <= 0 {
+ panic(fmt.Sprintf("SourceConfig.HotKeys must be >= 1. Got: %v", b.cfg.HotKeys))
+ }
+ if b.cfg.HotKeyFraction <= 0 {
+ panic(fmt.Sprintf("SourceConfig.HotKeyFraction must be >= 1. Got: %v", b.cfg.HotKeyFraction))
Review comment:
HotKeyFraction is supposed to be a floating point number from 0 to 1, right?
##########
File path: sdks/go/pkg/beam/io/synthetic/source.go
##########
@@ -262,8 +281,10 @@ func (b *SourceConfigBuilder) BuildFromJSON(jsonData []byte) SourceConfig {
// synthetic source. It should be created via a SourceConfigBuilder, not by
// directly initializing it (the fields are public to allow encoding).
type SourceConfig struct {
- NumElements int `json:"num_records"`
- InitialSplits int `json:"initial_splits"`
- KeySize int `json:"key_size"`
- ValueSize int `json:"value_size"`
+ NumElements int `json:"num_records"`
+ InitialSplits int `json:"initial_splits"`
+ HotKeys int `json:"hot_keys"`
Review comment:
I think you forgot about setters for your new variables. Please add them, because right now, we can only set them via JSON.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 506671)
Time Spent: 50m (was: 40m)
> Load Tests for Go SDK
> ---------------------
>
> Key: BEAM-11075
> URL: https://issues.apache.org/jira/browse/BEAM-11075
> Project: Beam
> Issue Type: Test
> Components: sdk-go, testing
> Reporter: Kamil Wasilewski
> Assignee: Kamil Wasilewski
> Priority: P3
> Time Spent: 50m
> Remaining Estimate: 0h
>
> We have Load Tests for Python and Java SDKs[1], but we are missing the ones for Go SDK.
> Tests to be done:
> * ParDo
> * Combine
> * coGBK
> * GBK
> * Side Input
> The tests should run on Dataflow and Flink. The tests should be using synthetic source and be running in batch mode.
> [1]http://metrics.beam.apache.org/dashboards/f/OtXje1iGz/performance-tests-metrics
--
This message was sent by Atlassian Jira
(v8.3.4#803005)