-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-11148. Update DataNode to use StorageLocationChecker at startup. #162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Change-Id: I664e5c719921ef5e0891bc392f37ee67639a8660
} | ||
|
||
if (storageLocationChecker != null) { | ||
storageLocationChecker.shutdownAndWait(10, TimeUnit.SECONDS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something for you to consider, most of the other threads seems to shutdown much quicker, do we need to wait 10 seconds ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. We can exit much quicker since a stalled disk check need not block shutdown at all. I'll change it to 1 second.
} | ||
|
||
@Test(timeout = 30000) | ||
public void testDataDirValidation() throws Throwable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presuming that we will have an equivalent or better test using storageLocationChecker ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes there's new tests in TestStorageLocationChecker added by HDFS-11119 which cover this better with different combinations of failed/healthy storages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, pending Jenkins and if you think 10 seconds for the datanode shutdown is ok.
Also fix unit tests flagged by Jenkins.
…le ordering of StorageLocations at startup.
Arpit, thanks for updating the patch. +1. LGTM. |
This issue has been fixed. Closing this PR as well. |
This is not the solution posted in SAMZA-1228. For now, we are moving jmxserver lifecycle to be within the container. Ideally, it should be within the Streamprocessor so that the job coordinator can also be associated with the same instance. Author: Navina Ramesh <navina@apache.org> Reviewers: Xinyu Liu <xiliu@linkedin.com>, Prateek Maheshwari <pmaheshw@linkedin.com> Closes apache#162 from navina/SAMZA-1228
Change-Id: I664e5c719921ef5e0891bc392f37ee67639a8660