You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spark.apache.org by Prajwal Tuladhar <pr...@infynyxx.com> on 2016/06/17 00:39:36 UTC
Spark internal Logging trait potential thread unsafe
Hi,
The way log instance inside Logger trait is current being initialized
doesn't seem to be thread safe [1]. Current implementation only guarantees
initializeLogIfNecessary() is initialized in lazy + thread safe way.
Is there a reason why it can't be just: [2]
@transient private lazy val log_ : Logger = {
initializeLogIfNecessary(false)
LoggerFactory.getLogger(logName)
}
And with that initializeLogIfNecessary() can be called without double
locking.
--
--
Cheers,
Praj
[1]
https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/internal/Logging.scala#L44-L50
[2]
https://github.com/apache/spark/blob/8ef3399aff04bf8b7ab294c0f55bcf195995842b/core/src/main/scala/org/apache/spark/internal/Logging.scala#L35
Re: Spark internal Logging trait potential thread unsafe
Posted by Prajwal Tuladhar <pr...@infynyxx.com>.
Created a JIRA issue https://issues.apache.org/jira/browse/SPARK-16131 and
PR @ https://github.com/apache/spark/pull/13842
On Fri, Jun 17, 2016 at 5:19 AM, Sean Owen <so...@cloudera.com> wrote:
> I think that's OK to change, yes. I don't see why it's necessary to
> init log_ the way it is now. initializeLogIfNecessary() has a purpose
> though.
>
> On Fri, Jun 17, 2016 at 2:39 AM, Prajwal Tuladhar <pr...@infynyxx.com>
> wrote:
> > Hi,
> >
> > The way log instance inside Logger trait is current being initialized
> > doesn't seem to be thread safe [1]. Current implementation only
> guarantees
> > initializeLogIfNecessary() is initialized in lazy + thread safe way.
> >
> > Is there a reason why it can't be just: [2]
> >
> > @transient private lazy val log_ : Logger = {
> > initializeLogIfNecessary(false)
> > LoggerFactory.getLogger(logName)
> > }
> >
> >
> > And with that initializeLogIfNecessary() can be called without double
> > locking.
> >
> > --
> > --
> > Cheers,
> > Praj
> >
> > [1]
> >
> https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/internal/Logging.scala#L44-L50
> > [2]
> >
> https://github.com/apache/spark/blob/8ef3399aff04bf8b7ab294c0f55bcf195995842b/core/src/main/scala/org/apache/spark/internal/Logging.scala#L35
>
--
--
Cheers,
Praj
Re: Spark internal Logging trait potential thread unsafe
Posted by Sean Owen <so...@cloudera.com>.
I think that's OK to change, yes. I don't see why it's necessary to
init log_ the way it is now. initializeLogIfNecessary() has a purpose
though.
On Fri, Jun 17, 2016 at 2:39 AM, Prajwal Tuladhar <pr...@infynyxx.com> wrote:
> Hi,
>
> The way log instance inside Logger trait is current being initialized
> doesn't seem to be thread safe [1]. Current implementation only guarantees
> initializeLogIfNecessary() is initialized in lazy + thread safe way.
>
> Is there a reason why it can't be just: [2]
>
> @transient private lazy val log_ : Logger = {
> initializeLogIfNecessary(false)
> LoggerFactory.getLogger(logName)
> }
>
>
> And with that initializeLogIfNecessary() can be called without double
> locking.
>
> --
> --
> Cheers,
> Praj
>
> [1]
> https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/internal/Logging.scala#L44-L50
> [2]
> https://github.com/apache/spark/blob/8ef3399aff04bf8b7ab294c0f55bcf195995842b/core/src/main/scala/org/apache/spark/internal/Logging.scala#L35
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@spark.apache.org
For additional commands, e-mail: dev-help@spark.apache.org