[FLINK-1486] add print method for prefixing a user defined string

[mxm]

• extend API to include a print(String sinkIdentifier) method
• change PrintingOutputformat to include the sink identifier
• if appropriate, print sink identifier and task id

[rmetzger]

Looks good.

[fhueske]

I think it would be nice to have some kind of hierarchical structure of the output such as:
$sinkName:$taskId > $outputValue
That would give the name of the sink first followed by the sink's task id, and finally behind the >prompt the actual output. Wouldn't that also make output parsing easier?

Looks good otherwise.

[mxm]

Good idea. So we would print $taskId > $outputValue if the user did not supply a string and $string:$taskId > $outputValue otherwise. If the parallelization degree is 1, we would just print $string > $outputValue if a string was supplied.

[fhueske]

Sounds good to me!

[mxm]

To make it a bit more explicit what is sink identifier and what is the task identifier (especially when just one of the two are printed), I prefixed the sink identifier with "sinkId" and the task identifier with "taskId".

[uce]

I think this is very valuable. I've tried it out and it looks good to me.

Personally, I would prefer the shorter versions proposed by Fabian. If we don't differentiate between parallelism 1 and > 1 we wouldn't have to worry about cases where just one is printed.

But I'm fine with your current solution as well.

[fhueske]

+1 for conciseness.

[mxm]

I've updated the pull request. I decided to implement the concise method:

sinkId:taskId> output  <- sink id provided, parallelism > 1
sinkId> output         <- sink id provided, parallelism == 1
taskId> output         <- no sink id provided, parallelism > 1
output                 <- no sink id provided, parallelism == 1

If no objections, I will merge this tomorrow.

[fhueske]

LGTM

[mxm]

I've added documentation for the new print method. Will merge later on.

[StephanEwen]

This may come a bit late (given that this is merged now), but I did not think of it before:

When we change the printing to happen on the client console (which we should, IMHO), we will probably realize it via collect().foreach(print) or so. In that case, there it gets executed immediately and there is always only one sink active.

Does this change still make sense then?

[fhueske]

I think you are right. If there's only one sink active, there is no need for a sink identifier.

[mxm]

Do we want to break backwards compatibility or include a new method for printing on the client? After all, printing on the workers is a useful tool to debug the dataflow of a program.

[StephanEwen]

Can you think of a case where printing on the client is worse than printing on worker?

[StephanEwen]

From what I have seen, most people expect print() to actually go to the client.
It would be a break of backwards compatibility, agreed. Maybe one of the few that are necessary.

[mxm]

@StephanEwen Don't think printing on the client can be worse if the output still contains information about the producers (e.g. by a task id). IMO, a sink identifier could still make sense when you make multiple calls to print and want to distinguish easily between the outputs.

[StephanEwen]

Ah, okay. You mean we have two methods:

1. print() which just prints everything onto the client
2. printWithTaskid(), which prefixes lines with the task ID?

[mxm]

Just saying that a prefix helps to identify output, even if everything is printed on the client. Additionally, including the task id in the output can be useful for debugging.

[StephanEwen]

Okay, let's just try and not make this too confusing for users. Do we need all three versions?

1. print()
2. printWithParallelId()
3. printWithPrefix()

[mxm]

Yes, it should be simple for the user. It makes sense to have one print method which just prints the output on the client. In addition, we could have another advanced print method which prints a prefix and optionally the task id.

• print()
• print(String prefix, boolean includeParallelID)