Zoltan Chovan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24527 )

Change subject: KUDU-3790: Fix NPE in HdrHistogramAccumulator.isZero()
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/24527/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/HdrHistogramAccumulator.scala
File 
java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/HdrHistogramAccumulator.scala:

http://gerrit.cloudera.org:8080/#/c/24527/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/HdrHistogramAccumulator.scala@42
PS1, Line 42:   override def isZero: Boolean = {
> q: If `histogram` can be null due to Kryo/unsafe-publication (the same scen
Yes, intentional scoping.
isZero() and toString() are the only methods Spark's heartbeater thread 
invokes, and that thread did not construct or deserialize the accumulator, so 
it has no happens-before edge with it and can observe an unpublished (null) 
field. copy(), reset(), add(), merge(), and value only run on the task-owning 
executor thread (which deserialized the closure) or the driver, both of which 
have happens-before with construction/deserialization and therefore always see 
a fully-published histogram.

I added a comment above copy() as an explanation


http://gerrit.cloudera.org:8080/#/c/24527/1/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/HdrHistogramAccumulator.scala@86
PS1, Line 86:  * exists.
> q: `innerHistogram` is a `var` read without synchronization in the new `isZ
honestly I didn't think of this, so good catch :)

I added volatile and some additional comments



--
To view, visit http://gerrit.cloudera.org:8080/24527
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d556aaf1051464303f35281d1d698b20d5ef20
Gerrit-Change-Number: 24527
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Chovan <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Gabriella Lotz <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Reviewer: Zoltan Martonka <[email protected]>
Gerrit-Comment-Date: Tue, 30 Jun 2026 16:44:06 +0000
Gerrit-HasComments: Yes

Reply via email to