bbeaudreault commented on PR #4853: URL: https://github.com/apache/hbase/pull/4853#issuecomment-1315875037
I've been looking into how these Tasks are created. It seems like all `MonitoredTask` are created through `TaskMonitor.createStatus`. All of the `createStatus` overloads take a description, so the description is always set on the MonitoredTask just after creation. For example: ``` MonitoredTask stat = new MonitoredTaskImpl(enableJournal); stat.setDescription(description); ``` In terms of usages, there are a few usages of getStatus and getDescription, mostly assuming non-null (except one which does a null check). If something were to accidentally break the convention of always calling setDescription, we'd have a similar failure on our hands. Since these classes are all IA.Private, I think we should change the constructor of `MonitoredTaskImpl` to take a description, and then set a default value of `"unset"` (or something to that effect) for status rather than the implicit default of null. This way it should be clear when viewing such a task that it never reached it's first call of `setStatus` While here, I think we should consider adding a null check in `setStatus` and `setDescription`, to avoid programming errors in the future re-introducing null states. Thoughts? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
