[ZEPPELIN-6282] Add null check condition

[proceane]

What is this PR for?

This PR was created to prevent an error that occurs in the InterpreterSetting.getInterpreterGroup method when groupId is null and is retrieved from the Set.
Normally, the groupId parameter does not seem to be passed as null.
I found it when tested service shutdown(#4965).

What type of PR is it?

Improvement

Todos

• - Add null check condition

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-6282

How should this be tested?

Screenshots (if appropriate)

Questions:

• Does the license files need to update? No
• Is there breaking changes for older versions? No
• Does this needs documentation? No

[ParkGyeongTae]

As I understand it, groupId should never be null under normal conditions, but it might become null in abnormal cases. Is there a way to reproduce such a scenario in tests?

[ParkGyeongTae]

Could you add a warn log before returning in the null check so that this situation is more visible?

[proceane]

Returning null could cause an NPE in the code currently calling this method (e.g., SessionConfInterpreter:53).
Therefore, I think it would be better to either not return null when groupId is null, or to modify the part that uses this method.

[ParkGyeongTae]

I agree with your point, and I’d like to ask for a code update.
I found that ManagedInterpreterGroup getInterpreterGroup(String groupId) is used in two places: line 386 of InterpreterSettingManager.java and line 53 of SessionConfInterpreter.java.

In InterpreterSettingManager.java, null is already handled properly, but in SessionConfInterpreter.java it is not. Could you please update the code to handle null in that part as well?

[proceane]

@ParkGyeongTae Thanks for review.
In my case, the problem occurred when I set it as follows.

1. When the ZeppelinServer shutdown method is executed, the shardServiceLocator.shutdown() method is executed.
2. Set the value of the zeppelin.search.enable option to false in the Recovery Test (if this value is true, an error occurs first in LuceneSearch, so it cannot be reproduced)

If you set it up as above and run the recovery test, it seems that a problem occurs where the groupId is set to null in the interpreter setting section when restarting the server.
As I mentioned at the beginning, this doesn't usually happen.

[ParkGyeongTae]

I was able to reproduce this issue, and I confirmed that the changes fix the problem.

[ParkGyeongTae]

Merged into master and branch-0.12