ZkBucketDataAccessor always complete scheduled GC

[GrantPSpencer]

Issues

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

#2872
When a write occurs, ZkBucketDataAccessor schedules a GC task to execute 60s (default ttl) from when the task was submitted. If there already exists a GC task, updateGCTimer attempts to cancel it reschedule it for 60s from then.

With this current logic, if the time between subsequent writes is never slower than the ttl (60s), then a GC task will never be executed - the task will constantly be scheduled and then cancelled.

Description

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

ZkBucket GC's are no longer rescheduled if another write occurs.
deleteStaleVersions has been refactored to determine recent version at method execution time (rather than passing currentVersion as a parameter)
This should guarantee that a GC will occur within TTL delay (60) seconds from any write.

Tests

• The following tests are written for this issue:

testGCScheduler

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

(If CI test fails due to known issue, please specify the issue and test PR locally. Then copy & paste the result of "mvn test" to here.)

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"

Code Quality

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

[junkaixue]

If we throw exception, the gc thread will be break, right? Shall we just log it and let it continue? Or we should stop the entire Accessor...

[GrantPSpencer]

I believe this try block should catch it, the one we are submitting to the GC thread:

         try {
            _gcTaskFutureMap.remove(rootPath);
            deleteStaleVersions(rootPath);
          } catch (Exception ex) {
            LOG.error("Failed to delete the stale versions.", ex);
          }
        }, _versionTTLms, TimeUnit.MILLISECONDS));

My understanding is that this should prevent the gc thread from crashing and just log the exception

compressedBucketRead will still throw the exception though

[GrantPSpencer]

Pull request approved @junkaixue
Commit message: ZkBucketDataAccessor always complete scheduled GC