HDFS-15875. Check whether file is being truncated before truncate #2746
Conversation
|
@ayushtkn Could you please help to review this? Thanks |
There was a problem hiding this comment.
Thanx @ferhui for the fix, The prod changes LGTM. Minor stuff in the test.
Apart, I see there are bunch of sleeps, can you give a check if the amount can be reduced by means of any config, or if there is any scope of using GenericTestUtil.waitFor in the test, just to ensure this test doesn't go flaky, and do increase the test timeout also a bit.
| try { | ||
| fs.truncate(p, data.length - 2); | ||
| } catch (IOException e) { | ||
| //GenericTestUtils.assertExceptionContains("is being truncated.", e); | ||
| } |
There was a problem hiding this comment.
Can use LambdaTestUtils.
LambdaTestUtils.intercept(RemoteException.class,
"/testTruncateTwiceTogether/file is being truncated",
() -> fs.truncate(p, data.length - 2));
| ThreadLocalRandom.current().nextBytes(data); | ||
| writeContents(data, data.length, p); | ||
|
|
||
| DataNodeFaultInjector originInjector = DataNodeFaultInjector.get(); |
| // Bigger than soft lease period. | ||
| Thread.sleep(65000); | ||
| } catch (InterruptedException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
No need to print the trace, if possible add Exception to method signature and remove this try-catch, else ignore the exception
|
@ayushtkn Thanks for review ! Will commit the fix soon! |
| fail("Truncate must fail since a truncate is already in progress."); | ||
| } catch (IOException expected) { | ||
| GenericTestUtils.assertExceptionContains( | ||
| "Failed to TRUNCATE_FILE", expected); | ||
| "/dir/testTruncateFailure is being truncated", expected); | ||
| } |
There was a problem hiding this comment.
What is the difference between this case and ours? Why doesn't this exception triggers for our case? and can we accommodate our fix to the check throwing this exception
There was a problem hiding this comment.
@ayushtkn Thanks for review!
This case happens with the same lease holder when enter recoverLeaseInternal. And our fix throw exception before enter recoverLeaseInternal. Our UT has 2 clients with different client names.
Will upload fix to throw the same exception.
|
@ayushtkn Any further questions? Thanks! |
|
@ayushtkn Thanks for review ! |
) (cherry picked from commit 6a55bae)
) (cherry picked from commit 6a55bae)
) (cherry picked from commit 6a55bae)
…truncate (apache#2746) (merge request !328)
…truncate (apache#2746) (cherry picked from commit 6a55bae) Change-Id: I0de89a5fd355f06e630ecfa59c91c1899d3d376d (cherry picked from commit d13eb53)
NOTICE
Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute