Fix SegmentZKMetadata time handling #7375
Conversation
|
Check-style won't let you do the inline assignment. I got red flag as well so had to cast twice. |
|
Thanks for fixing getTimeInterval() |
| TimeUnit timeUnit = TimeUnit.valueOf(_simpleFields.get(Segment.TIME_UNIT)); | ||
| return new Interval(timeUnit.toMillis(Long.parseLong(startTimeString)), | ||
| timeUnit.toMillis(Long.parseLong(endTimeString))); | ||
| long startTimeMs = getStartTimeMs(); |
There was a problem hiding this comment.
Seems like it is deprecated, and my IDE did not find any callers. If so, may be just remove?
There was a problem hiding this comment.
Will remove all the deprecated ones in a separate PR so that they are easier to track
| // NOTE: Need to check whether the start time is positive because some old segment ZK metadata contains negative | ||
| // start time and null time unit | ||
| long startTime; | ||
| if (startTimeString != null && (startTime = Long.parseLong(startTimeString)) > 0) { |
There was a problem hiding this comment.
It seems the build isn't happy about inner assignment. Maybe adding a private method to extract the field helps?
Jackie-Jiang
force-pushed
the
segment_zk_metadata_time_handling
branch
from
7311f69
to
23af1f7
Compare
Codecov Report
@@ Coverage Diff @@
## master #7375 +/- ##
=============================================
- Coverage 71.72% 30.46% -41.26%
+ Complexity 3309 88 -3221
=============================================
Files 1515 1506 -9
Lines 75008 74664 -344
Branches 10910 10874 -36
=============================================
- Hits 53798 22749 -31049
- Misses 17581 49909 +32328
+ Partials 3629 2006 -1623
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
On top of #7372, fix the
getTimeInterval()and minor performance improvement. Also add some comments for future reference