[FLINK-2425]Provide access to task manager configuration from RuntimeEnvironment

[sachingoel0101]

Also fixes [FLINK-2426]: Define an UnmodifiableConfiguration class which doesn't allow modifications to the underlying configuration object.

[StephanEwen]

If we want to unmodifiable configuration to be usable like any other configuration, it needs to be a subclass that overrides all mutating methods.

To make sure that is safe, it should also have a test that sees that all "set*" methods from the superclass are overridden.

[sachingoel0101]

Added test to verify all setter methods are overridden by the UnmodifiableConfiguration class.

[StephanEwen]

I think that silently failing is not a good idea in this case. Silent failures always keep people wondering why things don't behave like they expect. It needs to fail with an exception here.

Also, let's make the UnmodifiableConfiguration very lightweight, so that creating it has little overhead and footprint. Best way to do this is have it actually wrap the configuration, and have all the "getX()" methods delegate to the config, while all the "setX()" methods fail with an exception.

[StephanEwen]

For the test it is important that it catches the case where someone adds a method (for example "setStringArray()") to the base class and does not override it in the superclass.

We could also catch that easier, if we make the "setInternal()" method package private and override it in the UnmodifiableConfiguration to throw an exception.

[sachingoel0101]

...have it actually wrap the configuration, and have all the "getX()" methods delegate to the config,
while all the "setX()" methods fail with an exception.

That was my earlier solution [which is gone now because I rebased and force pushed :( ]. You're right though. It'd certainly make the initialization easier, but adds an additional calling overhead for every getter method [which are the most frequent. Initialization will be done exactly once]. IMO that's not a good idea.

I think that silently failing is not a good idea in this case. Silent failures always keep people >wondering why things don't behave like they expect. It needs to fail with an exception here.

That's easy enough! :') I just wasn't sure.

For the test it is important that it catches the case where someone adds a method (for example >"setStringArray()") to the base class and does not override it in the superclass.

The assumption being that any modifiers will be named set*?

[StephanEwen]

The earlier version had the Unmodifiable Config not as a subclass, which is crucial. Adding into the config is probably good enough, no need to over optimize this, I guess.

The assumption that mutating methods start with "set" or "add" is fair, IMHO.

[StephanEwen]

Looks good, modulo some styles...

• No need to override all "set*" methods, when the internal set method is overridden.
• Final modifiers should be used for new members where possible, consistent with the remaining code
• Let's not add new Scala classes. We decided that in the long run, it would be good to have the runtime pure Java again.

I need this quite soon, so let me take this over...

[sachingoel0101]

Ah. Okay. You can fix the remaining things.
This build also fails on the StreamCheckpointingITCase. I'll file another JIRA ticket for that. I've observed that twice.

[sachingoel0101]

@StephanEwen, since this has been merged, what do you think of exposing access to task manager configuration to the runtime context? This would open up a lot of possibilities by just modifying the TaskManagerRuntimeInfo class for any new information we need to pass on. There's a JIRA ticket for exposing the attempt number for example. It'd fix that right away.

[sachingoel0101]

It should however be called TaskRuntimeInfo in such a case, since it is being passed on to every task individually anyway.

[mxm]

@sachingoel0101 Since this has been merged, could you close the PR?