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

Updated ExportQ and CFM to use new ObserverProvider API #128

Closed
wants to merge 8 commits into
from

Conversation

Projects
None yet
3 participants
@keith-turner
Contributor

keith-turner commented Apr 21, 2017

No description provided.

Show outdated Hide outdated docs/accumulo-export-queue.md
@@ -19,8 +19,8 @@ limitations under the License.
## Background
The [Export Queue Recipe][1] provides a generic foundation for building export mechanism to any
external data store. The [AccumuloExporter] provides an implementation of this recipe for
Accumulo. The [AccumuloExporter] is located the `fluo-recipes-accumulo` module and provides the
external data store. The [AccumuloConsumer] provides an export consumer for writing to

This comment has been minimized.

@keith-turner

keith-turner Apr 25, 2017

Contributor

I do not like the name AccumuloConsumer, any suggestions?

@keith-turner

keith-turner Apr 25, 2017

Contributor

I do not like the name AccumuloConsumer, any suggestions?

This comment has been minimized.

@ctubbsii

ctubbsii Apr 25, 2017

Member

Maybe AccumuloReceiver or AccumuloIngester?

@ctubbsii

ctubbsii Apr 25, 2017

Member

Maybe AccumuloReceiver or AccumuloIngester?

This comment has been minimized.

@keith-turner

keith-turner Apr 25, 2017

Contributor

A bit of background. The name I like most is AccumuloExporter. I like this name because it implies sending things out from Fluo. However this name is currently taken by a class that's deprecated in this PR.

The super type for AccumuloConsumer is ExportConsumer (which is a new type in this PR).

@keith-turner

keith-turner Apr 25, 2017

Contributor

A bit of background. The name I like most is AccumuloExporter. I like this name because it implies sending things out from Fluo. However this name is currently taken by a class that's deprecated in this PR.

The super type for AccumuloConsumer is ExportConsumer (which is a new type in this PR).

This comment has been minimized.

@keith-turner

keith-turner Apr 25, 2017

Contributor

A bit more background. A lot of these changes were motivated by the desire to allow using lambdas after changes in Fluo allowed this. To support lambdas, the following happened.

  • Exporter was an abstract class and could not support lambdas, so was deprecated and replaced with ExportConsumer. ExportConsumer is a functional interface and lambdas can be assigned to it.
  • AccumuloExporter extended Exporter and was deprecated. It was replaced with AccumuloConsumer (which implements ExportConsumer) and AccumuloTranslator. AccumuloTranslator is functional interface and therefore lambdas can be assigned to it.

I really like the names Exporter and AccumuloExporter, but they can't support lambdas.

@keith-turner

keith-turner Apr 25, 2017

Contributor

A bit more background. A lot of these changes were motivated by the desire to allow using lambdas after changes in Fluo allowed this. To support lambdas, the following happened.

  • Exporter was an abstract class and could not support lambdas, so was deprecated and replaced with ExportConsumer. ExportConsumer is a functional interface and lambdas can be assigned to it.
  • AccumuloExporter extended Exporter and was deprecated. It was replaced with AccumuloConsumer (which implements ExportConsumer) and AccumuloTranslator. AccumuloTranslator is functional interface and therefore lambdas can be assigned to it.

I really like the names Exporter and AccumuloExporter, but they can't support lambdas.

This comment has been minimized.

@mikewalch

mikewalch Apr 25, 2017

Member

You could keep AccumuloExporter name but move to a new package

@mikewalch

mikewalch Apr 25, 2017

Member

You could keep AccumuloExporter name but move to a new package

Show outdated Hide outdated docs/accumulo-export-queue.md
@@ -19,8 +19,8 @@ limitations under the License.
## Background
The [Export Queue Recipe][1] provides a generic foundation for building export mechanism to any
external data store. The [AccumuloExporter] provides an implementation of this recipe for
Accumulo. The [AccumuloExporter] is located the `fluo-recipes-accumulo` module and provides the
external data store. The [AccumuloConsumer] provides an export consumer for writing to

This comment has been minimized.

@mikewalch

mikewalch Apr 25, 2017

Member

You could keep AccumuloExporter name but move to a new package

@mikewalch

mikewalch Apr 25, 2017

Member

You could keep AccumuloExporter name but move to a new package

// Set application properties for the collision free map. These properties are read later by
// observers.
CollisionFreeMap.configure(fluoConfig, cfmOpts);

This comment has been minimized.

@mikewalch

mikewalch Apr 25, 2017

Member

CollisionFreeMap could be named CombineQueue or Combiner

@mikewalch

mikewalch Apr 25, 2017

Member

CollisionFreeMap could be named CombineQueue or Combiner

combine updates that were queued by `DocumentObserver`. The collision free map
will process a batch of keys, calling the combiner for each key. When finished
processing a batch, it will call the update observer `WordCountObserver`.
Each collision free map has two extension points, a [combiner][ICombiner] and a [value

This comment has been minimized.

@mikewalch

mikewalch Apr 25, 2017

Member

Could be ValueCombiner and ValueObserver

@mikewalch

mikewalch Apr 25, 2017

Member

Could be ValueCombiner and ValueObserver

* the License.
*/
package org.apache.fluo.recipes.core.map;

This comment has been minimized.

@mikewalch

mikewalch Apr 25, 2017

Member

This could be in org.apache.fluo.recipes.core.combiner

@mikewalch

mikewalch Apr 25, 2017

Member

This could be in org.apache.fluo.recipes.core.combiner

*
* @since 1.1.0
*/
public interface ICombiner<K, V> {

This comment has been minimized.

@mikewalch

mikewalch Apr 25, 2017

Member

Could be called ValueCombiner

@mikewalch

mikewalch Apr 25, 2017

Member

Could be called ValueCombiner

* the License.
*/
package org.apache.fluo.recipes.core.map;

This comment has been minimized.

@mikewalch

mikewalch Apr 25, 2017

Member

could be in combiner or combine package

@mikewalch

mikewalch Apr 25, 2017

Member

could be in combiner or combine package

@keith-turner

This comment has been minimized.

Show comment
Hide comment
@keith-turner

keith-turner Apr 28, 2017

Contributor

I am going to resubmit this PR with the CollisionFreeMap deprecated and renamed to CombineQueue. The CombineQueue will only support the new Observer API.

Contributor

keith-turner commented Apr 28, 2017

I am going to resubmit this PR with the CollisionFreeMap deprecated and renamed to CombineQueue. The CombineQueue will only support the new Observer API.

@keith-turner keith-turner modified the milestone: 1.1.0 Jun 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment