Fix ZkBucketDataAccessor failure due to concurrent modification.

[jiajunwang]

Issues

• My PR addresses the following Helix issues and references them in the PR description:

#767

Description

• Here are some details about my PR, including screenshots of any UI changes:

Concurrent modification causes two issues.

1. Regular GC task fails due to concurrent list modification and the stale versions are not removed at all.
2. If, by coincident, there is a newer version in the list other then the current version, then because of the modification of the list inside the loop, the final element (the newer version) won't be filtered but being left in the to-be-removed list. Then the GC task removes the most recent version. For example,
   a) Input, current version "2"
   b) Children = [1, 2, 3]
   c) The task avoids checking "2", so the list for loop is: [1, 3]
   d) When checking "1", it is removed from the list. So the list becomes [3]. Then the loop ends, because the first item has already been looped from the for iteration perspective.
   e) The version to be removed is "3"!

This PR fixes the issue by avoiding concurrent modifications. Also, it simplifies the logic so as to reduce the pending GC tasks.
The test is also updated accordingly.

Tests

• The following tests are written for this issue:

TestZkBucketDataAccessor

• The following is the result of the "mvn test" command on the appropriate module:

[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] TestJobQueueCleanUp.testJobQueueAutoCleanUp:111 Sets differ: expected [testJobQueueAutoCleanUp_JOB8, testJobQueueAutoCleanUp_JOB7, testJobQueueAutoCleanUp_JOB6, testJobQueueAutoCleanUp_JOB5, testJobQueueAutoCleanUp_JOB9] but got [testJobQueueAutoCleanUp_JOB4, testJobQueueAutoCleanUp_JOB8, testJobQueueAutoCleanUp_JOB7, testJobQueueAutoCleanUp_JOB6, testJobQueueAutoCleanUp_JOB5, testJobQueueAutoCleanUp_JOB9]
[INFO]
[ERROR] Tests run: 1145, Failures: 1, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:19 h
[INFO] Finished at: 2020-06-19T19:27:46-07:00
[INFO] ------------------------------------------------------------------------

Rerun

[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 15.293 s - in org.apache.helix.integration.task.TestJobQueueCleanUp
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 21.621 s
[INFO] Finished at: 2020-06-19T21:53:23-07:00
[INFO] ------------------------------------------------------------------------

Commits

• My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Documentation (Optional)

• In case of new functionality, my PR adds documentation in the following wiki page:

(Link the GitHub wiki you added)

Code Quality

• My diff has been formatted using helix-style.xml
  (helix-style-intellij.xml if IntelliJ IDE is used)

[narendly]

Do we really need to cancel? What if that causes incomplete deletion of stale versions? I think we should let it queue up?

[jiajunwang]

This cancel will only cancel the pending tasks. For the pending tasks that are queued up, they will only cause extra children list read. The later operation will be the same. And if there is risk of incomplete deleting, then that concern lives no matter we let it queued up or not. That issue (if exists) should be addressed separately.

[jiajunwang]

This PR is ready to be merged, approved by @narendly