Skip to content

HADOOP-17728. HDFS-16033. Fix issue of the StatisticsDataReferenceCleaner cleanUp #3042

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.

Sign up for GitHub

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

Merged
liuml07 merged 3 commits into from
Jun 4, 2021
Merged

HADOOP-17728. HDFS-16033. Fix issue of the StatisticsDataReferenceCleaner cleanUp #3042

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1ec603
fix issue
May 21, 2021
6cf7c77
fix checkstyle
May 22, 2021
8b59d92
update
May 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4004,12 +4004,14 @@ public void cleanUp() {
* Background action to act on references being removed.
*/
private static class StatisticsDataReferenceCleaner implements Runnable {
private static int REF_QUEUE_POLL_TIMEOUT = 100;

@steveloughran steveloughran May 23, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's going to be waking every 100 milliseconds, demanding cpu time etc etc. If there has to be a timeout, it needs to be something less disruptive, like 100 seconds.

What would happen if that was the case?

@yikf yikf May 25, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

En, We should take this time into consideration. If it is too long, many resources need to be recycled in a certain period of time, which may lead to low recycling efficiency.

we can refer this: referenceQueue timeput

@Override
public void run() {
while (!Thread.interrupted()) {
try {
StatisticsDataReference ref =
(StatisticsDataReference)STATS_DATA_REF_QUEUE.remove();

@yikf yikf May 21, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewer, quote JDK ReferenceQueue#:

    public Reference<? extends T> remove(long timeout)
        throws IllegalArgumentException, InterruptedException
    {
        if (timeout < 0) {
            throw new IllegalArgumentException("Negative timeout value");
        }
        synchronized (lock) {
            Reference<? extends T> r = reallyPoll();
            if (r != null) return r;
            long start = (timeout == 0) ? 0 : System.nanoTime();
            for (;;) {
                lock.wait(timeout);
                r = reallyPoll();
                if (r != null) return r;
                if (timeout != 0) {
                    long end = System.nanoTime();
                    timeout -= (end - start) / 1000_000;
                    if (timeout <= 0) return null;
                    start = end;
                }
            }
        }
    }

    /**
     * Removes the next reference object in this queue, blocking until one
     * becomes available.
     *
     * @return A reference object, blocking until one becomes available
     * @throws  InterruptedException  If the wait is interrupted
     */
    public Reference<? extends T> remove() throws InterruptedException {
        return remove(0);
    }

(StatisticsDataReference)STATS_DATA_REF_QUEUE.
remove(REF_QUEUE_POLL_TIMEOUT);
ref.cleanUp();
} catch (InterruptedException ie) {
LOGGER.warn("Cleaner thread interrupted, will stop", ie);
Expand Down