Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Add counters to default summers #571

Merged
merged 9 commits into from Feb 11, 2015
Merged

Add counters to default summers #571

merged 9 commits into from Feb 11, 2015

Conversation

MansurAshraf
Copy link
Contributor

This will not compile till new version of Algebird is pulled in but its using features from twitter/algebird#373

@ianoc
Copy link
Collaborator

ianoc commented Dec 15, 2014

Builds are all failed?

@MansurAshraf
Copy link
Contributor Author

yeah because its using Algebird version thats not released yet. I did a local publish to get it working

ianoc
ianoc reviewed Jan 20, 2015
View reviewed changes
summingbird-storm/src/main/scala/com/twitter/summingbird/storm/BuildSummer.scala
val nodeName = dag.getNodeName(node)
val cacheSize = storm.getOrElse(dag, node, DEFAULT_FM_CACHE)
logger.info("[{}] cacheSize lowerbound: {}", nodeName, cacheSize.lowerBound)

val memoryCounter = counter(jobID, Name(nodeName), Name("memory"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good, though i think a comment for each of these to describe them would be good. What does memory mean here?

@MansurAshraf
Copy link
Contributor Author

build failing on 2.10.4 due to serialization exception on com.twitter.summingbird.Group. I am unable to reproduce this locally and all tests are passing on 2.10.4.

Caused by: java.lang.ClassNotFoundException: com.twitter.summingbird.Group
    at java.net.URLClassLoader$1.run(URLClassLoader.java:366)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:355)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:354)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:425)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:358)
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:274)
    at java.io.ObjectInputStream.resolveClass(ObjectInputStream.java:625)

@MansurAshraf
Copy link
Contributor Author

Group is on classpath so I doubt its a classpath issue.

@johnynek
Copy link
Collaborator

johnynek commented Feb 5, 2015

This is also this failure: https://travis-ci.org/twitter/summingbird/jobs/49233461#L2698

But this failure is strange.

@isnotinvain
Copy link
Contributor

I think I might just merge this into #565 since it's not going to work w/o this I think

@isnotinvain isnotinvain mentioned this pull request Feb 5, 2015
Merged
@isnotinvain isnotinvain merged commit 96f540e into twitter:develop Feb 11, 2015
isnotinvain added a commit that referenced this pull request Feb 11, 2015
@isnotinvain
Merge pull request #565 from twitter/removeScala2.9.3
21d193d 
Use latest bijection, algebird, scalding, storehaus, chill, and tormenta, remove scala 2.9 support, merge #571 and #553
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@MansurAshraf @ianoc @johnynek @isnotinvain