Wrap undo in try block

[Manno15]

In response to comment from @keith-turner. I added a try block around my previous change on #1746. With the unreserveNamespace call in the finally block.

[Manno15]

Should I catch the Exception as well? I noticed on other spots that we didn't so I wasn't sure.

[ctubbsii]

This is good, but there's a similar issue in the ChooseDir undo method, where it doesn't first check that the tableInfo.getInitialSplitSize() > 0, which means it can get an NPE. It might be good to make that change as part of this PR also, if you're okay with that, @Manno15 .

There's also a similar issue in FinishCreateTable.cleanupSplitFiles, which can cause the table creation to fail, even if the table was, in fact, created. Perhaps it'd be better to just log that error, instead of failing and rolling back the entire table creation? I'm not sure. What do you think, @keith-turner ?

[keith-turner]

There's also a similar issue in FinishCreateTable.cleanupSplitFiles, which can cause the table creation to fail, even if the table was, in fact, created. Perhaps it'd be better to just log that error, instead of failing and rolling back the entire table creation?

Logging an error in that case does seem better.

[keith-turner]

Should I catch the Exception as well? I noticed on other spots that we didn't so I wasn't sure.

@Manno15 based on the comment by @ctubbsii I am thinking maybe in all of the places where this file is deleted that if it fails the exception should be caught and an error or warning logged. So an exception is not thrown out of the operation if delete fails.

[Manno15]

I'll add the requested changes.

[Manno15]

There's also a similar issue in FinishCreateTable.cleanupSplitFiles, which can cause the table creation to fail, even if the table was, in fact, created.

FinishCreateTable.call has the tableInfo.getInitialSplitSize() > 0 check before calling FinsihCreateTable.cleanupSplitFiles. I don't think that one has the issue as the other two. I can still catch the exceptions just in case so we don't run into this predicament.

[ctubbsii]

Thanks @Manno15 . This looks good to me. Since you're a committer now, you should be able to merge it yourself. I recommend merging from the GitHub UI in the pull request. You should use the "Squash and merge" option from the dropdown, and clean up your log message as part of the process. Let me know if you have questions.