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

Use duration to match configuration parameter #842

Merged
merged 1 commit into from
Jul 24, 2019
Merged

Use duration to match configuration parameter #842

merged 1 commit into from
Jul 24, 2019

Conversation

chazdnato
Copy link
Contributor

The configuration for this timeout is config.getNewConsumerPollTimeoutSeconds(); which is also used in SecorKafkaMessageIterator.java:55 as such.

This was causing issues, whereby the monitor poll would quickly timeout. Instead of having inconsistent usage, this seems the simplest approach.

Note that the .poll() method in org.apache.kafka.clients.consumer.KafkaConsumer is described in milliseconds.

The configuration for this timeout is `config.getNewConsumerPollTimeoutSeconds();` which is also used in `SecorKafkaMessageIterator.java:55` as such.

This was causing issues, whereby the monitor poll would quickly timeout. Instead of having inconsistent usage, this seems the simplest approach.

Note that the `.poll()` method in `org.apache.kafka.clients.consumer.KafkaConsumer` is described in milliseconds.
@HenryCaiHaiying
Copy link
Contributor

Change looks straightforward to me, do you have time to look at the test failure?

@chazdnato
Copy link
Contributor Author

Change looks straightforward to me, do you have time to look at the test failure?

Unfortunately, it looks like this PR was submitted right around when github had an outage. Three of the tests failed with a 500 error trying to download s3cmd from github. The other failure appears to be due to the test taking too long to run. Are you able to re-run? I can submit a nonce change to re-trigger a run.

@HenryCaiHaiying
Copy link
Contributor

I am rebuilding those tests now.

@chazdnato
Copy link
Contributor Author

I see the builds failed again, but unclear if this is due to my change or how long the build takes:

The job exceeded the maximum log length, and has been terminated.

@HenryCaiHaiying
Copy link
Contributor

Actually the real error is this:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 49.114 s
[INFO] Finished at: 2019-07-22T18:38:16Z
[INFO] Final Memory: 32M/332M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-resources-plugin:3.1.0:resources (default-resources) on project secor: Execution default-resources of goal org.apache.maven.plugins:maven-resources-plugin:3.1.0:resources failed: Plugin org.apache.maven.plugins:maven-resources-plugin:3.1.0 or one of its dependencies could not be resolved: Could not transfer artifact org.sonatype.sisu:sisu-guice:jar:noaop:2.1.7 from/to google-maven-central (https://maven-central.storage-download.googleapis.com/repos/central/data/): GET request of: org/sonatype/sisu/sisu-guice/2.1.7/sisu-guice-2.1.7-noaop.jar from google-maven-central failed: Connection reset -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginResolutionException
make: *** [unit] Error 1
The command "make unit" exited with 2.

@chazdnato
Copy link
Contributor Author

:/ Unclear how this relates to my change. Or how to get past this.

@HenryCaiHaiying
Copy link
Contributor

That jar was actually in the traditional maven central: https://repo1.maven.org/maven2/org/sonatype/sisu/sisu-guice/2.1.7/

Not sure when/why travis is now using google maven central.

@HenryCaiHaiying
Copy link
Contributor

Looks like this switch happened on 6/28: https://changelog.travis-ci.com/new-maven-central-mirror-is-in-use-106758

@HenryCaiHaiying
Copy link
Contributor

HenryCaiHaiying commented Jul 23, 2019

Sent an email to google maven admin: [email protected] to see whether they can add that artifact

@HenryCaiHaiying
Copy link
Contributor

Got reply from google, probably will be fixed in a few more days:

Les Vogel [email protected] Les Vogel [email protected] 2:29 PM (3 minutes ago)    
Les Vogel [email protected]
to me, gcs-maven-mirror to me, gcs-maven-mirror
to me, gcs-maven-mirror

Yep - I have a job fixing some syncs running now - it's at 10 days so far, it should finish in a few more days.

I see it in the source we are syncing from. (An SSD on one of my machines)

sisu-guice-2.1.7.jar sisu-guice-2.1.7-javadoc.jar sisu-guice-2.1.7-noaop.jar sisu-guice-2.1.7.pom sisu-guice-2.1.7-sources.jar
sisu-guice-2.1.7.jar.asc sisu-guice-2.1.7-javadoc.jar.asc sisu-guice-2.1.7-noaop.jar.asc sisu-guice-2.1.7.pom.asc sisu-guice-2.1.7-sources.jar.asc
sisu-guice-2.1.7.jar.asc.md5 sisu-guice-2.1.7-javadoc.jar.asc.md5 sisu-guice-2.1.7-noaop.jar.asc.md5 sisu-guice-2.1.7.pom.asc.md5 sisu-guice-2.1.7-sources.jar.asc.md5
sisu-guice-2.1.7.jar.asc.sha1 sisu-guice-2.1.7-javadoc.jar.asc.sha1 sisu-guice-2.1.7-noaop.jar.asc.sha1 sisu-guice-2.1.7.pom.asc.sha1 sisu-guice-2.1.7-sources.jar.asc.sha1
sisu-guice-2.1.7.jar.md5 sisu-guice-2.1.7-javadoc.jar.md5 sisu-guice-2.1.7-noaop.jar.md5 sisu-guice-2.1.7.pom.md5 sisu-guice-2.1.7-sources.jar.md5
sisu-guice-2.1.7.jar.sha1 sisu-guice-2.1.7-javadoc.jar.sha1 sisu-guice-2.1.7-noaop.jar.sha1 sisu-guice-2.1.7.pom.sha1 sisu-guice-2.1.7-sources.jar.sha1

Though it's a very early version, they are on 4.2.0 now. Not sure why it wasn't on the source Sonatype originally gave me, up to your mentioning it, I only knew of some checksum's missing from older archives.

@chazdnato
Copy link
Contributor Author

@HenryCaiHaiying Thank you! I appreciate your deep dive into this, for a simple fix!

@HenryCaiHaiying
Copy link
Contributor

Looks like the google maven central issue is fixed. Now hit the second problem on 'log length is too long' on Travis-CI build, fixing this by: #847

@HenryCaiHaiying HenryCaiHaiying merged commit b90f4a9 into pinterest:master Jul 24, 2019
@HenryCaiHaiying
Copy link
Contributor

All the build issues are fixed, merging this PR in.

@chazdnato
Copy link
Contributor Author

Thank you for all your help!

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