[GitHub] [hadoop] simbadzina commented on pull request #5145: HDFS-16847: Prevents StateStoreFileSystemImpl from committing tmp file after encountering an IOException.

[GitBox <gi...@apache.org>]

simbadzina commented on PR #5145:
URL: https://github.com/apache/hadoop/pull/5145#issuecomment-1320761423

   > Thanks @simbadzina for your contribution.
   > 
   > @goiri - Do you think we can add a specific UT for this change? Also, I can see the current test has been updated to check it.
   > 
   > I am +1 overall.
   
   Thanks for the reviews @ashutoshcipher and @goiri . In this review I didn't update tests.
   I spent some time trying to add a test but I couldn't find an elegant way to do use.
   I had two ideas
   1) Adding a fault injector to StateStoreFileBaseImpl . And then use the injector to throw an en exception during the write of the file.
   2) Mocking StateStoreFileSystemImpl and throwing an exception when the serializeString(..) method is called. 
   
   (1) felt like overkill. I couldn't get (2) to work because serializeString() is a protected method in a parent class.
   If you have any thoughts on a test we can add I'd greatly appreciate pointers.


-- 
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: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org