Update to guice 4.0 by drcrallen · Pull Request #1608 · apache/druid

drcrallen · 2015-08-07T18:43:36Z

Mark a lot of @Provides methods as final since guice 4.0 disallows overriding them

xvrl · 2015-08-07T21:19:14Z

should we mark this method as to be used for testing only

xvrl · 2015-08-07T21:20:22Z

minor comment, but LGTM otherwise

nishantmonu51 · 2015-08-07T23:27:46Z

👍

xvrl · 2015-08-08T00:48:20Z

@drcrallen we should also update guice version in druid-api while we're at it for consistency, and update druid-api version

drcrallen · 2015-08-08T00:50:12Z

@xvrl : druid-io/druid-api#51

xvrl · 2015-08-10T16:53:19Z

Sounds good, we can merge this as soon as druid-api with updated guice is released, and druid-api version has been updated in this PR.

- Mark a lot of `@Provides` methods as final since guice 4.0 disallows overriding them

drcrallen · 2015-08-10T20:58:27Z

build is expected to fail until the release for druid-api finishes.

drcrallen · 2015-08-10T21:49:16Z

@xvrl travis is passing here.

Update to guice 4.0

himanshug · 2015-08-12T17:25:52Z

@drcrallen not certain but this PR seems to be causing following exception in hadoop batch ingestion (I'm using hadoop-2.6)
(com.google.inject.spi.ProviderInstanceBinding.getUserSuppliedProvider() was introduced in guice-4.0)

1) Error injecting method, java.lang.NoSuchMethodError: com.google.inject.spi.ProviderInstanceBinding.getUserSuppliedProvider()Ljavax/inject/Provider;
  at com.google.inject.multibindings.Multibinder$RealMultibinder.initialize(Multibinder.java:324)
  at io.druid.guice.PolyBind.optionBinder(PolyBind.java:113)
  while locating java.util.Set<java.util.Map$Entry<java.lang.String, com.google.inject.Provider<io.druid.metadata.MetadataSegmentPublisher>>>
  at com.google.inject.multibindings.MapBinder$RealMapBinder$RealProviderMapProvider.initialize(MapBinder.java:461)
  at io.druid.guice.PolyBind.optionBinder(PolyBind.java:113)
Caused by: java.lang.NoSuchMethodError: com.google.inject.spi.ProviderInstanceBinding.getUserSuppliedProvider()Ljavax/inject/Provider;
    at com.google.inject.multibindings.Indexer.visit(Indexer.java:139)
    at com.google.inject.multibindings.Indexer.visit(Indexer.java:43)
    at com.google.inject.internal.ProviderInstanceBindingImpl.acceptTargetVisitor(ProviderInstanceBindingImpl.java:62)
    at com.google.inject.multibindings.Multibinder$RealMultibinder.initialize(Multibinder.java:332)
    at com.google.inject.multibindings.Multibinder$RealMultibinder$$FastClassByGuice$$94efefc8.invoke(<generated>)
    at com.google.inject.internal.cglib.reflect.$FastMethod.invoke(FastMethod.java:53)
    at com.google.inject.internal.SingleMethodInjector$1.invoke(SingleMethodInjector.java:56)
    at com.google.inject.internal.SingleMethodInjector.inject(SingleMethodInjector.java:90)
    at com.google.inject.internal.MembersInjectorImpl.injectMembers(MembersInjectorImpl.java:110)
    at com.google.inject.internal.MembersInjectorImpl$1.call(MembersInjectorImpl.java:75)
    at com.google.inject.internal.MembersInjectorImpl$1.call(MembersInjectorImpl.java:73)
    at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1031)
    at com.google.inject.internal.MembersInjectorImpl.injectAndNotify(MembersInjectorImpl.java:73)
    at com.google.inject.internal.Initializer$InjectableReference.get(Initializer.java:147)
    at com.google.inject.internal.InternalFactoryToProviderAdapter.get(InternalFactoryToProviderAdapter.java:40)
    at com.google.inject.internal.InjectorImpl$4$1.call(InjectorImpl.java:978)
    at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1031)
    at com.google.inject.internal.InjectorImpl$4.get(InjectorImpl.java:974)
    at com.google.inject.spi.ProviderLookup$1.get(ProviderLookup.java:89)
    at com.google.inject.spi.ProviderLookup$1.get(ProviderLookup.java:89)
    at com.google.inject.spi.ProviderLookup$1.get(ProviderLookup.java:89)
    at com.google.inject.multibindings.MapBinder$RealMapBinder$RealProviderMapProvider.initialize(MapBinder.java:469)
    at com.google.inject.multibindings.MapBinder$RealMapBinder$RealProviderMapProvider$$FastClassByGuice$$7fff4848.invoke(<generated>)
    at com.google.inject.internal.cglib.reflect.$FastMethod.invoke(FastMethod.java:53)
    at com.google.inject.internal.SingleMethodInjector$1.invoke(SingleMethodInjector.java:56)
    at com.google.inject.internal.SingleMethodInjector.inject(SingleMethodInjector.java:90)
    at com.google.inject.internal.MembersInjectorImpl.injectMembers(MembersInjectorImpl.java:110)
    at com.google.inject.internal.MembersInjectorImpl$1.call(MembersInjectorImpl.java:75)
    at com.google.inject.internal.MembersInjectorImpl$1.call(MembersInjectorImpl.java:73)
    at com.google.inject.internal.InjectorImpl.callInContext(InjectorImpl.java:1024)
    at com.google.inject.internal.MembersInjectorImpl.injectAndNotify(MembersInjectorImpl.java:73)
    at com.google.inject.internal.Initializer$InjectableReference.get(Initializer.java:147)
    at com.google.inject.internal.Initializer.injectAll(Initializer.java:92)
    at com.google.inject.internal.InternalInjectorCreator.injectDynamically(InternalInjectorCreator.java:173)
    at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:109)
    at com.google.inject.Guice.createInjector(Guice.java:95)
    at com.google.inject.Guice.createInjector(Guice.java:72)
    at com.google.inject.Guice.createInjector(Guice.java:62)
    at io.druid.initialization.Initialization.makeInjectorWithModules(Initialization.java:412)
    at io.druid.indexer.HadoopDruidIndexerConfig.<clinit>(HadoopDruidIndexerConfig.java:93)
    at io.druid.indexer.DetermineHashedPartitionsJob$DetermineHashedPartitionsPartitioner.setConf(DetermineHashedPartitionsJob.java:393)
    at org.apache.hadoop.util.ReflectionUtils.setConf(ReflectionUtils.java:73)
    at org.apache.hadoop.util.ReflectionUtils.newInstance(ReflectionUtils.java:133)
    at org.apache.hadoop.mapred.MapTask$NewOutputCollector.<init>(MapTask.java:698)
    at org.apache.hadoop.mapred.MapTask.runNewMapper(MapTask.java:767)
    at org.apache.hadoop.mapred.MapTask.run(MapTask.java:341)
    at org.apache.hadoop.mapred.YarnChild$2.run(YarnChild.java:163)
    at java.security.AccessController.doPrivileged(Native Method)
    at javax.security.auth.Subject.doAs(Subject.java:422)
    at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1628)
    at org.apache.hadoop.mapred.YarnChild.main(YarnChild.java:158)

are you successfully being able to run hadoop ingestion after this update?

drcrallen · 2015-08-12T17:44:20Z

@himanshug Using my simple test of index hadoop index-test.json through my IntelliJ setup seems to work fine. Still investigating to see if the indexing service has something weird with this update

drcrallen · 2015-08-12T18:06:53Z

@himanshug Specifying "hadoopCoordinates": "org.apache.hadoop:hadoop-client:2.6.0" seems to be working locally. for the indexing service

himanshug · 2015-08-12T18:18:30Z

@drcrallen hmmm, not sure yet, but I have a hadoop-2.6 cluster deployed . ran druid processes with hadoop classpath output and hdfs-storage on classpath. doing the ingestion using HadoopIndexTask via overlord and it failed on me, removing the 2 commits from this PR solved the problem.

xvrl · 2015-08-12T18:47:19Z

@himanshug can you make sure nothing in your build or extensions is pulling in an older version of guice? This seems like somewhere on your classpath an older version of guice got pulled in.

himanshug · 2015-08-12T18:53:57Z

@xvrl that is true, "hadoop classpath" would include guice jar bundled with hadoop deployment, which will be guice-3.0 .
I think, probably I'm facing the issue because both guice-4.0 and guice-3.0 are ending up being on the classpath. In this case I'm using druid-services-0.8.1-SNAPSHOT-selfcontained.jar which will have guice-4.0 classes inside it.

drcrallen · 2015-08-14T16:27:22Z

@himanshug Showed up in our dogfood cluster. Reverting until IT can be fixed to catch issue.

himanshug · 2015-08-14T16:51:12Z

@drcrallen makes sense to revert till we figure out the root cause.

drcrallen closed this Aug 7, 2015

drcrallen reopened this Aug 7, 2015

xvrl reviewed Aug 7, 2015
View reviewed changes

drcrallen force-pushed the guice40 branch from c253bcc to 84fc9ab Compare August 7, 2015 21:21

xvrl modified the milestone: 0.8.1 Aug 10, 2015

drcrallen added 2 commits August 10, 2015 13:57

Update to guice 4.0

7e61216

- Mark a lot of `@Provides` methods as final since guice 4.0 disallows overriding them

Bump druid-api to 0.3.10 to include guice 4.0 update

c8c8169

drcrallen force-pushed the guice40 branch from 22afdac to c8c8169 Compare August 10, 2015 20:58

xvrl added a commit that referenced this pull request Aug 10, 2015

Merge pull request #1608 from metamx/guice40

fadd8a4

Update to guice 4.0

xvrl merged commit fadd8a4 into apache:master Aug 10, 2015

xvrl deleted the guice40 branch August 10, 2015 22:00

drcrallen mentioned this pull request Aug 14, 2015

Revert "Update to guice 4.0" #1628

Merged

drcrallen mentioned this pull request Jul 6, 2016

Update to guice-4.1.0. #3222

Merged

Conversation

drcrallen commented Aug 7, 2015

Uh oh!

xvrl Aug 7, 2015

Choose a reason for hiding this comment

Uh oh!

drcrallen Aug 7, 2015

Choose a reason for hiding this comment

Uh oh!

xvrl commented Aug 7, 2015

Uh oh!

nishantmonu51 commented Aug 7, 2015

Uh oh!

xvrl commented Aug 8, 2015

Uh oh!

drcrallen commented Aug 8, 2015

Uh oh!

xvrl commented Aug 10, 2015

Uh oh!

drcrallen commented Aug 10, 2015

Uh oh!

drcrallen commented Aug 10, 2015

Uh oh!

himanshug commented Aug 12, 2015

Uh oh!

drcrallen commented Aug 12, 2015

Uh oh!

drcrallen commented Aug 12, 2015

Uh oh!

himanshug commented Aug 12, 2015

Uh oh!

xvrl commented Aug 12, 2015

Uh oh!

himanshug commented Aug 12, 2015

Uh oh!

drcrallen commented Aug 14, 2015

Uh oh!

himanshug commented Aug 14, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants