[BEAM-1715] Fix Python WordCount on Dataflow Mismatch #2244

markflyhigh · 2017-03-14T20:29:09Z

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

Make sure the PR title is formatted like:
[BEAM-<Jira issue #>] Description of pull request
Make sure tests pass via mvn clean verify. (Even better, enable
Travis-CI on your fork and ensure the whole test matrix passes).
Replace <Jira issue #> in the title with the actual Jira issue
number, if there is one.
If this contribution is large, please file an Apache
Individual Contributor License Agreement.

Add more logs for debugging
Add sleep before verification in order to give more time to have files ready on FS, especially GCS.

markflyhigh · 2017-03-14T20:36:19Z

aaltay · 2017-03-14T20:44:55Z

sdks/python/apache_beam/tests/pipeline_verifiers_test.py

  @patch.object(ChannelFactory, 'glob')
-  def test_file_checksum_matcher_read_failed(self, mock_glob):
+  def test_file_checksum_matcher_read_failed(self, mock_glob, _):


Are you sure this is the right order and not mock_time, mock_glob?

yes, otherwise mock_glob will be wrong and following asserts must failed.

aaltay · 2017-03-14T20:56:48Z

sdks/python/apache_beam/tests/pipeline_verifiers.py

    for path in matched_path:
      with ChannelFactory.open(path, 'r') as f:
        for line in f:
          read_lines.append(line)
    return read_lines

  def _matches(self, _):
+    # Wait to have output file ready on FS
+    wait_time = 20


Can yo make this configurable, I would assume that not every FS will need such a wait.

make sense. I can move it to object constructor.

Since this is problem coming from GCS, I think it may better to be configured by test writer in IT when they use GCS as output storage. I'd rather leave default sleep to 0.

coveralls · 2017-03-14T21:09:38Z

Coverage decreased (-0.003%) to 70.177% when pulling 0a76a23 on markflyhigh:python-file-verifier-inconsistency into cc12fd3 on apache:master.

asfbot · 2017-03-14T21:10:22Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8398/
--none--

markflyhigh · 2017-03-15T00:09:01Z

Run Python PostCommit

coveralls · 2017-03-15T00:45:10Z

Coverage decreased (-0.001%) to 70.179% when pulling ba31373 on markflyhigh:python-file-verifier-inconsistency into cc12fd3 on apache:master.

asfbot · 2017-03-15T00:45:39Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8419/
--none--

asfbot · 2017-03-15T02:20:20Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PostCommit_Python_Verify/1509/
--none--

aaltay · 2017-03-15T17:19:46Z

Do you know why the postcommit test failed?

markflyhigh · 2017-03-15T19:15:29Z

From the log in BEAM-1715, the first time when matcher read from gcs only got 3179 lines which is less than expected (4784 lines). I guess the output file is not fully staged at that time, so waiting for few seconds will help.

markflyhigh · 2017-03-15T19:15:45Z

Run Python PostCommit

coveralls · 2017-03-15T19:43:34Z

Coverage increased (+0.002%) to 70.183% when pulling 08037d2 on markflyhigh:python-file-verifier-inconsistency into cc12fd3 on apache:master.

asfbot · 2017-03-15T19:44:33Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8456/
--none--

asfbot · 2017-03-15T21:06:42Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PostCommit_Python_Verify/1517/
--none--

markflyhigh · 2017-03-15T22:56:31Z

Run Python PostCommit

asfbot · 2017-03-16T02:02:55Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PostCommit_Python_Verify/1522/
--none--

markflyhigh · 2017-03-16T17:24:07Z

PTAL @aaltay

aaltay · 2017-03-16T18:42:04Z

sdks/python/apache_beam/examples/wordcount_it_test.py

@@ -43,9 +43,11 @@ def test_wordcount_it(self):
    output = '/'.join([test_pipeline.get_option('output'),
                       test_pipeline.get_option('job_name'),
                       'results'])
+    sleep_secs = int(test_pipeline.get_option('sleep_secs'))


Did you mean this sleep_secs to be an optional argument or a required one?

sleep_secs should be optional for wordcount_it_test. You are right, this will break test if no sleep_secs specified. Will fix

markflyhigh · 2017-03-17T00:07:23Z

Run Python PostCommit

coveralls · 2017-03-17T00:43:52Z

Coverage decreased (-0.03%) to 70.146% when pulling aefd83a on markflyhigh:python-file-verifier-inconsistency into cc12fd3 on apache:master.

asfbot · 2017-03-17T00:44:37Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8510/

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-runners-spark: 1

org.apache.beam.runners.spark.translation.streaming.ResumeFromCheckpointStreamingTest.testWithResume

--none--

asfbot · 2017-03-17T00:58:50Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PostCommit_Python_Verify/1534/
--none--

markflyhigh · 2017-03-17T01:32:22Z

PTAL @aaltay

aaltay · 2017-03-22T01:35:06Z

sdks/python/apache_beam/examples/wordcount_it_test.py

@@ -43,9 +43,12 @@ def test_wordcount_it(self):
    output = '/'.join([test_pipeline.get_option('output'),
                       test_pipeline.get_option('job_name'),
                       'results'])
+    arg_sleep_secs = test_pipeline.get_option('sleep_secs')
+    sleep_time = int(arg_sleep_secs) if arg_sleep_secs is not None else None


sleep_time -> sleep_secs for consistency.

aaltay · 2017-03-22T01:35:59Z

sdks/python/apache_beam/tests/pipeline_verifiers.py

+        self.sleep_secs = sleep_secs
+      else:
+        raise ValueError('Sleep seconds, if received, must be int. '
+                         'But received: %r' % sleep_secs)


consider adding the type information here, might be useful.

markflyhigh · 2017-03-22T22:51:54Z

PTAL @aaltay

asfbot · 2017-03-22T22:58:09Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8679/

Build result: FAILURE

[...truncated 580.74 KB...] at hudson.remoting.UserRequest.perform(UserRequest.java:153) at hudson.remoting.UserRequest.perform(UserRequest.java:50) at hudson.remoting.Request$2.run(Request.java:336) at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.maven.plugin.MojoFailureException: You have 1 Checkstyle violation. at org.apache.maven.plugin.checkstyle.CheckstyleViolationCheckMojo.execute(CheckstyleViolationCheckMojo.java:588) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-03-22T22:57:40.976 [ERROR] 2017-03-22T22:57:40.976 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-03-22T22:57:40.976 [ERROR] 2017-03-22T22:57:40.976 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-03-22T22:57:40.976 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-03-22T22:57:40.976 [ERROR] 2017-03-22T22:57:40.976 [ERROR] After correcting the problems, you can resume the build with the command2017-03-22T22:57:40.976 [ERROR] mvn -rf :beam-runners-core-javachannel stoppedSetting status of 0104081 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8679/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

aaltay · 2017-03-22T23:08:05Z

retest this please

asfbot · 2017-03-22T23:21:06Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8681/

Build result: FAILURE

[...truncated 826.43 KB...] at hudson.remoting.UserRequest.perform(UserRequest.java:153) at hudson.remoting.UserRequest.perform(UserRequest.java:50) at hudson.remoting.Request$2.run(Request.java:336) at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.maven.plugin.MojoFailureException: You have 1 Checkstyle violation. at org.apache.maven.plugin.checkstyle.CheckstyleViolationCheckMojo.execute(CheckstyleViolationCheckMojo.java:588) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-03-22T23:20:20.140 [ERROR] 2017-03-22T23:20:20.140 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-03-22T23:20:20.140 [ERROR] 2017-03-22T23:20:20.140 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-03-22T23:20:20.140 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-03-22T23:20:20.140 [ERROR] 2017-03-22T23:20:20.140 [ERROR] After correcting the problems, you can resume the build with the command2017-03-22T23:20:20.141 [ERROR] mvn -rf :beam-runners-core-javachannel stoppedSetting status of 0104081 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8681/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

coveralls · 2017-03-23T00:14:36Z

Coverage decreased (-0.003%) to 70.019% when pulling 3eaf3b4 on markflyhigh:python-file-verifier-inconsistency into eb347bf on apache:master.

asfbot · 2017-03-23T00:21:35Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8685/
--none--

coveralls · 2017-03-23T00:28:16Z

Coverage decreased (-0.006%) to 70.016% when pulling 3eaf3b4 on markflyhigh:python-file-verifier-inconsistency into eb347bf on apache:master.

asfbot · 2017-03-23T00:29:17Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/8687/
--none--

aaltay · 2017-03-23T00:33:04Z

LGTM

asfbot · 2017-03-23T00:34:25Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PostCommit_Python_Verify/1584/
--none--

aaltay reviewed Mar 14, 2017

View reviewed changes

aaltay reviewed Mar 16, 2017

View reviewed changes

aaltay reviewed Mar 22, 2017

View reviewed changes

markflyhigh closed this Mar 22, 2017

markflyhigh reopened this Mar 22, 2017

markflyhigh added 6 commits March 22, 2017 16:21

[BEAM-1715] Fix Python WordCount on Dataflow Mismatch

617222e

fixup! Remove unused import

169c08b

Make sleep time configurable from commandline

0fb4d7e

fixup! Fix error sleep_secs type

1bb3e53

fixup! Sleep in WordcountIT should be optionsal

ecf6c12

fixup! Improve Comments

3eaf3b4

markflyhigh force-pushed the python-file-verifier-inconsistency branch from 0104081 to 3eaf3b4 Compare March 22, 2017 23:33

asfgit closed this in 70efdd0 Mar 23, 2017

markflyhigh deleted the python-file-verifier-inconsistency branch March 23, 2017 00:36

[BEAM-1715] Fix Python WordCount on Dataflow Mismatch #2244

[BEAM-1715] Fix Python WordCount on Dataflow Mismatch #2244

Conversation

markflyhigh commented Mar 14, 2017 • edited

markflyhigh commented Mar 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markflyhigh Mar 14, 2017 • edited

Choose a reason for hiding this comment

coveralls commented Mar 14, 2017

asfbot commented Mar 14, 2017

markflyhigh commented Mar 15, 2017

coveralls commented Mar 15, 2017

asfbot commented Mar 15, 2017

asfbot commented Mar 15, 2017

aaltay commented Mar 15, 2017

markflyhigh commented Mar 15, 2017

markflyhigh commented Mar 15, 2017

coveralls commented Mar 15, 2017

asfbot commented Mar 15, 2017

asfbot commented Mar 15, 2017

markflyhigh commented Mar 15, 2017

asfbot commented Mar 16, 2017

markflyhigh commented Mar 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markflyhigh commented Mar 17, 2017

coveralls commented Mar 17, 2017

asfbot commented Mar 17, 2017

Failed Tests: 1

beam_PreCommit_Java_MavenInstall/org.apache.beam:beam-runners-spark: 1

asfbot commented Mar 17, 2017

markflyhigh commented Mar 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markflyhigh commented Mar 22, 2017

asfbot commented Mar 22, 2017

Build result: FAILURE

aaltay commented Mar 22, 2017

asfbot commented Mar 22, 2017

Build result: FAILURE

coveralls commented Mar 23, 2017

asfbot commented Mar 23, 2017

coveralls commented Mar 23, 2017

asfbot commented Mar 23, 2017

aaltay commented Mar 23, 2017

asfbot commented Mar 23, 2017

markflyhigh commented Mar 14, 2017 •

edited

markflyhigh Mar 14, 2017 •

edited