[GitHub] [spark] xkrogen commented on pull request #33116: [SPARK-35259][SHUFFLE] Rename ExternalBlockHandler Timer variables to remove incorrect millis suffix

2021-07-22 Thread GitBox
xkrogen commented on pull request #33116: URL: https://github.com/apache/spark/pull/33116#issuecomment-885045100 Pushed up new commit fixing the indentation. Looks like the GitHub Actions failures are due to the issues being worked on in #33475. The Jenkins build also failed

[GitHub] [spark] xkrogen commented on pull request #33116: [SPARK-35259][SHUFFLE] Rename ExternalBlockHandler Timer variables to remove incorrect millis suffix

2021-07-21 Thread GitBox
xkrogen commented on pull request #33116: URL: https://github.com/apache/spark/pull/33116#issuecomment-884487300 Pushed up a new commit fixing a few old references to milliseconds and adding testing for the `Timer.Context` API. Thanks for the review @Ngone51 ! -- This is an automated

[GitHub] [spark] xkrogen commented on pull request #33116: [SPARK-35259][SHUFFLE] Rename ExternalBlockHandler Timer variables to remove incorrect millis suffix

2021-07-20 Thread GitBox
xkrogen commented on pull request #33116: URL: https://github.com/apache/spark/pull/33116#issuecomment-883500849 @dongjoon-hyun @Ngone51 gentle ping, any thoughts on the latest diff? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to

[GitHub] [spark] xkrogen commented on pull request #33116: [SPARK-35259][SHUFFLE] Rename ExternalBlockHandler Timer variables to remove incorrect millis suffix

2021-07-15 Thread GitBox
xkrogen commented on pull request #33116: URL: https://github.com/apache/spark/pull/33116#issuecomment-880841663 Pushed up a new commit changing `TimerWithMillisecondSnapshots` to `TimerWithCustomTimeUnit`, which will accept a configurable time unit as a constructor parameter. I also

[GitHub] [spark] xkrogen commented on pull request #33116: [SPARK-35259][SHUFFLE] Rename ExternalBlockHandler Timer variables to remove incorrect millis suffix

2021-07-15 Thread GitBox
xkrogen commented on pull request #33116: URL: https://github.com/apache/spark/pull/33116#issuecomment-880807696 Ah, I see, I misunderstood what you meant. I thought you meant that the place where the values are _extracted_ from the Timer (e.g. YarnShuffleServiceMetrics) would supply a

[GitHub] [spark] xkrogen commented on pull request #33116: [SPARK-35259][SHUFFLE] Rename ExternalBlockHandler Timer variables to remove incorrect millis suffix

2021-07-14 Thread GitBox
xkrogen commented on pull request #33116: URL: https://github.com/apache/spark/pull/33116#issuecomment-880022892 > I think we can force the `TimerWithMillisecondSnapshots` to receive a time-unit parameter to indicate which time-unit it exposes. I don't think we can force callers to

[GitHub] [spark] xkrogen commented on pull request #33116: [SPARK-35259][SHUFFLE] Rename ExternalBlockHandler Timer variables to remove incorrect millis suffix

2021-07-12 Thread GitBox
xkrogen commented on pull request #33116: URL: https://github.com/apache/spark/pull/33116#issuecomment-878521045 Just put up a new diff which implements a custom `Timer` subclass, `TimerWithMillisecondSnapshots`, which acts the same as a normal `Timer` and stores nanoseconds internally,

[GitHub] [spark] xkrogen commented on pull request #33116: [SPARK-35259][SHUFFLE] Rename ExternalBlockHandler Timer variables to remove incorrect millis suffix

2021-07-01 Thread GitBox
xkrogen commented on pull request #33116: URL: https://github.com/apache/spark/pull/33116#issuecomment-872595353 @dongjoon-hyun updated per your comments, PTAL. I also updated the documentation to address the discrepancy. -- This is an automated message from the Apache Git Service. To