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

Scalding batch tests #296

Merged
merged 6 commits into from
Oct 14, 2013
Merged

Scalding batch tests #296

merged 6 commits into from
Oct 14, 2013

Conversation

johnynek
Copy link
Collaborator

The point of this code is to start testing scalding with a random batcher. Previously, we put everything in one batch so it was easy to write the test code (which needs more refactoring). Now, we generate a random batcher that tries up to 5 batches.

This exposed a lot of little assumptions in the testing, but no bugs in summingbird yet.

The next step is to use this better testing to verify optimizations to the scalding side don't break the logic.

First up: a better algorithm that puts all the heavy lifting into one Map/Reduce job (so run 100 batches in 1 MR step). Until we get template-tap support is scalding, after that step there will be a 100 concurrent Map-only jobs to write to the correct partitions, but that should be really fast, and will be solved when we close:

twitter/scalding#654

or

twitter/scalding#484

@johnynek
Copy link
Collaborator Author

Looks like that failure was a storm failure. Restarted. We need to get the false positives down on our failure detector. Maybe the stuff @ianoc did on the security manager is ready to merge.

@johnynek
Copy link
Collaborator Author

@jnievelt this was work I started before we do more optimizations of the scalding side. I want to have better confidence that we don't regress as we improve the planning.

@@ -79,8 +88,9 @@ class TestStore[K, V](store: String, inBatcher: Batcher, initBatch: BatchID, ini
}.toMap

// Call this after you compute to check the results of the
def lastToIterable(b: BatchID): Iterable[(K, V)] =
sourceToBuffer(batches(b)).toIterable.map { tup => tconv(new TupleEntry(tup)) }
def lastToIterable: Iterable[(K, V)] = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

drop the braces?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

:) yep. I saw that on github after I pushed. I had added them to get a val, I later removed.

ianoc added a commit that referenced this pull request Oct 14, 2013
@ianoc ianoc merged commit 1553f68 into develop Oct 14, 2013
@ianoc ianoc deleted the scalding-batch-tests branch October 14, 2013 18:40
snoble pushed a commit to snoble/summingbird that referenced this pull request Sep 8, 2017
…rsFromSB

Feature/migrate async summers from sb
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants