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

Make parallelism available to ProcessorSupplier.Context and Processor.Context #754

Merged
merged 3 commits into from Feb 28, 2018

Conversation

Projects
None yet
3 participants
@cangencer
Copy link
Collaborator

commented Feb 27, 2018

New additional fields of localParallelism and totalParallelism are now available in Processor and ProcessorSupplier level.

Also made them extend the upstream interfaces, so that gradually more information is available in each context as you drill down.

@cangencer cangencer added this to the 0.6 milestone Feb 27, 2018

@cangencer cangencer requested a review from viliam-durina Feb 27, 2018

@@ -254,12 +247,6 @@ default boolean finishSnapshotRestore() {
*/
int globalProcessorIndex();

This comment has been minimized.

Copy link
@viliam-durina

viliam-durina Feb 28, 2018

Contributor

We should document here that it's zero-based and that it's in the range 0 .. totalParallelism-1

*/
@Nonnull
JetInstance jetInstance();
interface Context extends ProcessorSupplier.Context {

/**
* Return a logger for the processor

This comment has been minimized.

Copy link
@viliam-durina

viliam-durina Feb 28, 2018

Contributor

The logger() is also in PS.Context, we can delete it here maybe. Is it only due to javadoc?
Also, the processingGuarantee and snapshottingEnabled methods can be pushed up to PMS level.

This comment has been minimized.

Copy link
@cangencer

cangencer Feb 28, 2018

Author Collaborator

Yes they end up all being the same thing. Only ProcCtx has the unique index so maybe we should just have two types rather than three?

@Nonnull DistributedFunction<ConsumerRecord<K, V>, T> projectionFn,
@Nonnull WatermarkGenerationParams<T> wmGenParams
) {
return new CloseableProcessorSupplier<StreamKafkaP>(() -> new StreamKafkaP<>(

This comment has been minimized.

Copy link
@viliam-durina

viliam-durina Feb 28, 2018

Contributor

We've lost the preferred local parallelism of 2 here.
And the explicit type parameter is not needed.

This comment has been minimized.

Copy link
@cangencer

cangencer Feb 28, 2018

Author Collaborator

I've moved the preferred parallelism to KafkaProcessors.streamKafkaP. Will fix the explicit type params.

This comment has been minimized.

Copy link
@viliam-durina

viliam-durina Feb 28, 2018

Contributor

ok, missed that

@cangencer cangencer force-pushed the cangencer:supplier-contexts branch from 4b42b29 to 4df3b95 Feb 28, 2018

@hazelcast hazelcast deleted a comment from devOpsHazelcast Feb 28, 2018

@hazelcast hazelcast deleted a comment from devOpsHazelcast Feb 28, 2018

@devOpsHazelcast

This comment has been minimized.

Copy link
Collaborator

commented Feb 28, 2018

Test PASSed.

@devOpsHazelcast

This comment has been minimized.

Copy link
Collaborator

commented Feb 28, 2018

Test PASSed.

@cangencer cangencer merged commit 4052344 into hazelcast:master Feb 28, 2018

1 check passed

default Build finished.
Details

@cangencer cangencer deleted the cangencer:supplier-contexts branch Feb 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.