Skip to content

Conversation

@kamilwu
Copy link
Contributor

@kamilwu kamilwu commented Jul 11, 2019

Based on the following proposal: https://s.apache.org/load-test-basic-operations


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- Build Status --- --- Build Status
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Build Status
Python Build Status
Build Status
--- Build Status
Build Status
Build Status --- --- Build Status

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website
Non-portable Build Status Build Status Build Status Build Status
Portable --- Build Status --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 12, 2019

Run Seed Job

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 12, 2019

Run Python Load Tests ParDo Dataflow Batch

1 similar comment
@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 12, 2019

Run Python Load Tests ParDo Dataflow Batch

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 12, 2019

Run Seed Job

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 12, 2019

Run Python Load Tests ParDo Dataflow Batch

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 12, 2019

Run Seed Job

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 12, 2019

Run Python Load Tests ParDo Dataflow Batch

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 12, 2019

@lgajowy Please take a look.
I had to increase the number of workers from 5 (as in the proposal) to 10, because the job had been running for longer than 2 hours. As a result, it was always aborted.

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 15, 2019

Run Seed Job

Copy link
Contributor

@lgajowy lgajowy left a comment

Choose a reason for hiding this comment

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

Added comments. Thanks!

jobProperties: [
job_name : 'load-tests-python-dataflow-batch-pardo-1-' + now,
project : 'apache-beam-testing',
temp_location : 'gs://temp-storage-for-perf-tests/smoketests',
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the correct temp location?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's correct, but 'gs://temp-storage-for-perf-tests/loadtests' seems to be used more often in load tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain why is this a problem? Did you experience any issues when using /loadtests?

Copy link
Contributor Author

@kamilwu kamilwu Jul 16, 2019

Choose a reason for hiding this comment

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

I think you misunderstood. It's not a problem. /smoketests was there accidentally, and I've already replaced it to /loadtests, which suits better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, we're seeing stale code in the thread. Thanks! :)

'"value_size": 90}\'',
iterations : 10,
number_of_counter_operations: 0,
number_of_counters : 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there 1 counter? According to the proposal the goal of this test is to check what is the inter operation overhead (not metrics). So there should be no counters. Counters are needed in tests #3 and #4 where we examine metrics overhead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The number_of_counter_operations is zero, so there is actually no metrics overhead.
But I guess this 1 counter can be misleading, so I'll change it.

iterations : 10,
number_of_counter_operations: 0,
number_of_counters : 1,
num_workers : 10,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we using 10 workers here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've already changed it to 5.

],
]}

def loadTestConfigurationManyCounters = { datasetName -> [
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think splitting the job to 2 jobs is the solution here. It is very weird that it takes so much time (above 2 hours) to run, whereas java needs 28 minutes to run all the tests (as we discussed offline). Could you investigate this a little bit more? Maybe the pipeline shape is not as we think it is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have double-checked the code and it seems fine. The problem is poor performance of metrics operations in Python — test case with 10 iterations and 100 counter operations needed almost 3 hours to complete.

The solution is to lower the number of iterations to 1, which makes the test way faster. Job split will be also unnecessary.

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 15, 2019

Run Python Load Tests ParDo Dataflow Batch

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 15, 2019

Run Seed Job

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 15, 2019

Run Python Load Tests ParDo Dataflow Batch

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 15, 2019

Run Seed Job

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 15, 2019

Run Python Load Tests ParDo Dataflow Batch

@kamilwu kamilwu force-pushed the pardo-jenkins branch 3 times, most recently from 74cdcdd to d58fbee Compare July 18, 2019 09:18
@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 18, 2019

Run Seed Job

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 18, 2019

Run Python Load Tests ParDo Dataflow Batch

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 22, 2019

Run Seed Job

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 22, 2019

Run Python Load Tests ParDo Dataflow Batch

Copy link
Contributor

@lgajowy lgajowy left a comment

Choose a reason for hiding this comment

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

Code LGTM. Please reorganize commit history and we're good to go. Remeber about adding "[BEAM-7502]" to each commit title.

Thanks!

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 22, 2019

@lgajowy It's done, commits are ready

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 23, 2019

Run Seed Job

@kamilwu
Copy link
Contributor Author

kamilwu commented Jul 23, 2019

Run Python Load Tests ParDo Dataflow Batch

@lgajowy lgajowy merged commit 1640133 into apache:master Jul 23, 2019
@kamilwu kamilwu deleted the pardo-jenkins branch July 23, 2019 09:54
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.

2 participants