Re: impala::Mutex

[Zoltan Borok-Nagy]

Thanks for the recommendation, the whole abseil library looks cool.

abls::Mutex is especially interesting, since it conceptually differs from
traditional mutexes. Although, this means a bit more work if we want to use
it, because it won't work as a drop-in replacement.

Zoli


On Tue, Oct 31, 2017 at 8:47 PM, Todd Lipcon  wrote:

> BTW I should have linked the following in my earlier email:
> https://abseil.io/about/design/mutex
>
> -Todd
>
> On Tue, Oct 31, 2017 at 10:21 AM, Todd Lipcon  wrote:
>
> > Yea, we use our own mutex which is a simple wrapper around pthreads, but
> > also adds some instrumentation and debugging.
> >
> > We have recently been thinking about switching to absl::Mutex from
> Google.
> > It has adaptive spinning, faster wake up for condition waiters, and a
> nice
> > API which avoids missed notify bugs. It also has the same or better
> > instrumentation support.
> >
> > If you are going to switch I'd take a look at that implementation as
> well.
> >
> > On Oct 31, 2017 10:12 AM, "Tim Armstrong" 
> wrote:
> >
> >> The main case where we still use boost::mutex is when it's pair with a
> >> condition variable - we don't have any condition variable that
> integrates
> >> with SpinLock.
> >>
> >> Mutex does provide some stronger fairness guarantees, I think, but afaik
> >> we
> >> don't rely on that anywhere.
> >>
> >> On Tue, Oct 31, 2017 at 9:54 AM, Daniel Hecht 
> >> wrote:
> >>
> >> > We have impala::SpinLock, which works with lock_guard, unique_lock,
> etc.
> >> > It's adaptive so it will block after spinning for a little bit, and
> >> seems
> >> > to work well in most cases. It's built on gutil, which also has some
> >> > tracing we could enable. Any reason not to stick with that?
> >> >
> >> > I haven't looked at Kudu's implementation in a while and am not
> opposed
> >> to
> >> > using it if there's a good reason to switch.  But I think maybe we
> >> should
> >> > first figure out how to better share code with Kudu so that we don't
> >> just
> >> > continue to fork general purpose utility code (which is what we're
> >> > currently doing).
> >> >
> >> > On Tue, Oct 31, 2017 at 9:47 AM, Zoltan Borok-Nagy <
> >> > borokna...@cloudera.com>
> >> > wrote:
> >> >
> >> > > Hi Everyone,
> >> > >
> >> > > I started to review the usage of synchronization primitives in
> Impala
> >> and
> >> > > created this JIRA issue: https://issues.apache.org/
> >> > jira/browse/IMPALA-6125
> >> > >
> >> > > After some chat with Tim, we talked about the possibilities of
> >> reducing
> >> > the
> >> > > dependence of boost. Since now we are using C++11, there are
> >> std::thread,
> >> > > std::mutex, and std::condition_variable in the standard library.
> >> > >
> >> > > On the other hand, the standard library likes to use exceptions in
> >> case
> >> > of
> >> > > errors, and AFAIK we don't like exceptions in Impala. Also, we
> already
> >> > have
> >> > > utility classes like ConditionVariable in the code base. In the
> >> kudu/util
> >> > > directory, there is a Mutex class which is more conform to the
> Impala
> >> > code
> >> > > conventions than boost and the standard library. It also checks the
> >> > > ownership of the mutex in debug mode, which can be useful for
> >> detecting
> >> > > bugs.
> >> > >
> >> > > Do you think it would be a good idea to create our own Mutex
> >> > implementation
> >> > > based on kudu/util/mutex?
> >> > >
> >> > > BR,
> >> > > Zoltan
> >> > >
> >> >
> >>
> >
>
>
> --
> Todd Lipcon
> Software Engineer, Cloudera
>

Re: impala::Mutex

[Todd Lipcon]

BTW I should have linked the following in my earlier email:
https://abseil.io/about/design/mutex

-Todd

On Tue, Oct 31, 2017 at 10:21 AM, Todd Lipcon  wrote:

> Yea, we use our own mutex which is a simple wrapper around pthreads, but
> also adds some instrumentation and debugging.
>
> We have recently been thinking about switching to absl::Mutex from Google.
> It has adaptive spinning, faster wake up for condition waiters, and a nice
> API which avoids missed notify bugs. It also has the same or better
> instrumentation support.
>
> If you are going to switch I'd take a look at that implementation as well.
>
> On Oct 31, 2017 10:12 AM, "Tim Armstrong"  wrote:
>
>> The main case where we still use boost::mutex is when it's pair with a
>> condition variable - we don't have any condition variable that integrates
>> with SpinLock.
>>
>> Mutex does provide some stronger fairness guarantees, I think, but afaik
>> we
>> don't rely on that anywhere.
>>
>> On Tue, Oct 31, 2017 at 9:54 AM, Daniel Hecht 
>> wrote:
>>
>> > We have impala::SpinLock, which works with lock_guard, unique_lock, etc.
>> > It's adaptive so it will block after spinning for a little bit, and
>> seems
>> > to work well in most cases. It's built on gutil, which also has some
>> > tracing we could enable. Any reason not to stick with that?
>> >
>> > I haven't looked at Kudu's implementation in a while and am not opposed
>> to
>> > using it if there's a good reason to switch.  But I think maybe we
>> should
>> > first figure out how to better share code with Kudu so that we don't
>> just
>> > continue to fork general purpose utility code (which is what we're
>> > currently doing).
>> >
>> > On Tue, Oct 31, 2017 at 9:47 AM, Zoltan Borok-Nagy <
>> > borokna...@cloudera.com>
>> > wrote:
>> >
>> > > Hi Everyone,
>> > >
>> > > I started to review the usage of synchronization primitives in Impala
>> and
>> > > created this JIRA issue: https://issues.apache.org/
>> > jira/browse/IMPALA-6125
>> > >
>> > > After some chat with Tim, we talked about the possibilities of
>> reducing
>> > the
>> > > dependence of boost. Since now we are using C++11, there are
>> std::thread,
>> > > std::mutex, and std::condition_variable in the standard library.
>> > >
>> > > On the other hand, the standard library likes to use exceptions in
>> case
>> > of
>> > > errors, and AFAIK we don't like exceptions in Impala. Also, we already
>> > have
>> > > utility classes like ConditionVariable in the code base. In the
>> kudu/util
>> > > directory, there is a Mutex class which is more conform to the Impala
>> > code
>> > > conventions than boost and the standard library. It also checks the
>> > > ownership of the mutex in debug mode, which can be useful for
>> detecting
>> > > bugs.
>> > >
>> > > Do you think it would be a good idea to create our own Mutex
>> > implementation
>> > > based on kudu/util/mutex?
>> > >
>> > > BR,
>> > > Zoltan
>> > >
>> >
>>
>


-- 
Todd Lipcon
Software Engineer, Cloudera

Re: impala::Mutex

[Todd Lipcon]

Yea, we use our own mutex which is a simple wrapper around pthreads, but
also adds some instrumentation and debugging.

We have recently been thinking about switching to absl::Mutex from Google.
It has adaptive spinning, faster wake up for condition waiters, and a nice
API which avoids missed notify bugs. It also has the same or better
instrumentation support.

If you are going to switch I'd take a look at that implementation as well.

On Oct 31, 2017 10:12 AM, "Tim Armstrong"  wrote:

> The main case where we still use boost::mutex is when it's pair with a
> condition variable - we don't have any condition variable that integrates
> with SpinLock.
>
> Mutex does provide some stronger fairness guarantees, I think, but afaik we
> don't rely on that anywhere.
>
> On Tue, Oct 31, 2017 at 9:54 AM, Daniel Hecht  wrote:
>
> > We have impala::SpinLock, which works with lock_guard, unique_lock, etc.
> > It's adaptive so it will block after spinning for a little bit, and seems
> > to work well in most cases. It's built on gutil, which also has some
> > tracing we could enable. Any reason not to stick with that?
> >
> > I haven't looked at Kudu's implementation in a while and am not opposed
> to
> > using it if there's a good reason to switch.  But I think maybe we should
> > first figure out how to better share code with Kudu so that we don't just
> > continue to fork general purpose utility code (which is what we're
> > currently doing).
> >
> > On Tue, Oct 31, 2017 at 9:47 AM, Zoltan Borok-Nagy <
> > borokna...@cloudera.com>
> > wrote:
> >
> > > Hi Everyone,
> > >
> > > I started to review the usage of synchronization primitives in Impala
> and
> > > created this JIRA issue: https://issues.apache.org/
> > jira/browse/IMPALA-6125
> > >
> > > After some chat with Tim, we talked about the possibilities of reducing
> > the
> > > dependence of boost. Since now we are using C++11, there are
> std::thread,
> > > std::mutex, and std::condition_variable in the standard library.
> > >
> > > On the other hand, the standard library likes to use exceptions in case
> > of
> > > errors, and AFAIK we don't like exceptions in Impala. Also, we already
> > have
> > > utility classes like ConditionVariable in the code base. In the
> kudu/util
> > > directory, there is a Mutex class which is more conform to the Impala
> > code
> > > conventions than boost and the standard library. It also checks the
> > > ownership of the mutex in debug mode, which can be useful for detecting
> > > bugs.
> > >
> > > Do you think it would be a good idea to create our own Mutex
> > implementation
> > > based on kudu/util/mutex?
> > >
> > > BR,
> > > Zoltan
> > >
> >
>

Re: impala::Mutex

[Tim Armstrong]

The main case where we still use boost::mutex is when it's pair with a
condition variable - we don't have any condition variable that integrates
with SpinLock.

Mutex does provide some stronger fairness guarantees, I think, but afaik we
don't rely on that anywhere.

On Tue, Oct 31, 2017 at 9:54 AM, Daniel Hecht  wrote:

> We have impala::SpinLock, which works with lock_guard, unique_lock, etc.
> It's adaptive so it will block after spinning for a little bit, and seems
> to work well in most cases. It's built on gutil, which also has some
> tracing we could enable. Any reason not to stick with that?
>
> I haven't looked at Kudu's implementation in a while and am not opposed to
> using it if there's a good reason to switch.  But I think maybe we should
> first figure out how to better share code with Kudu so that we don't just
> continue to fork general purpose utility code (which is what we're
> currently doing).
>
> On Tue, Oct 31, 2017 at 9:47 AM, Zoltan Borok-Nagy <
> borokna...@cloudera.com>
> wrote:
>
> > Hi Everyone,
> >
> > I started to review the usage of synchronization primitives in Impala and
> > created this JIRA issue: https://issues.apache.org/
> jira/browse/IMPALA-6125
> >
> > After some chat with Tim, we talked about the possibilities of reducing
> the
> > dependence of boost. Since now we are using C++11, there are std::thread,
> > std::mutex, and std::condition_variable in the standard library.
> >
> > On the other hand, the standard library likes to use exceptions in case
> of
> > errors, and AFAIK we don't like exceptions in Impala. Also, we already
> have
> > utility classes like ConditionVariable in the code base. In the kudu/util
> > directory, there is a Mutex class which is more conform to the Impala
> code
> > conventions than boost and the standard library. It also checks the
> > ownership of the mutex in debug mode, which can be useful for detecting
> > bugs.
> >
> > Do you think it would be a good idea to create our own Mutex
> implementation
> > based on kudu/util/mutex?
> >
> > BR,
> > Zoltan
> >
>

Re: impala::Mutex

[Daniel Hecht]

We have impala::SpinLock, which works with lock_guard, unique_lock, etc.
It's adaptive so it will block after spinning for a little bit, and seems
to work well in most cases. It's built on gutil, which also has some
tracing we could enable. Any reason not to stick with that?

I haven't looked at Kudu's implementation in a while and am not opposed to
using it if there's a good reason to switch.  But I think maybe we should
first figure out how to better share code with Kudu so that we don't just
continue to fork general purpose utility code (which is what we're
currently doing).

On Tue, Oct 31, 2017 at 9:47 AM, Zoltan Borok-Nagy 
wrote:

> Hi Everyone,
>
> I started to review the usage of synchronization primitives in Impala and
> created this JIRA issue: https://issues.apache.org/jira/browse/IMPALA-6125
>
> After some chat with Tim, we talked about the possibilities of reducing the
> dependence of boost. Since now we are using C++11, there are std::thread,
> std::mutex, and std::condition_variable in the standard library.
>
> On the other hand, the standard library likes to use exceptions in case of
> errors, and AFAIK we don't like exceptions in Impala. Also, we already have
> utility classes like ConditionVariable in the code base. In the kudu/util
> directory, there is a Mutex class which is more conform to the Impala code
> conventions than boost and the standard library. It also checks the
> ownership of the mutex in debug mode, which can be useful for detecting
> bugs.
>
> Do you think it would be a good idea to create our own Mutex implementation
> based on kudu/util/mutex?
>
> BR,
> Zoltan
>

impala::Mutex

[Zoltan Borok-Nagy]

Hi Everyone,

I started to review the usage of synchronization primitives in Impala and
created this JIRA issue: https://issues.apache.org/jira/browse/IMPALA-6125

After some chat with Tim, we talked about the possibilities of reducing the
dependence of boost. Since now we are using C++11, there are std::thread,
std::mutex, and std::condition_variable in the standard library.

On the other hand, the standard library likes to use exceptions in case of
errors, and AFAIK we don't like exceptions in Impala. Also, we already have
utility classes like ConditionVariable in the code base. In the kudu/util
directory, there is a Mutex class which is more conform to the Impala code
conventions than boost and the standard library. It also checks the
ownership of the mutex in debug mode, which can be useful for detecting
bugs.

Do you think it would be a good idea to create our own Mutex implementation
based on kudu/util/mutex?

BR,
Zoltan

impala::Mutex

[Zoltan Borok-Nagy]

Hi Everyone,

I started to review the usage of synchronization primitives in Impala and
created this JIRA issue: https://issues.apache.org/jira/browse/IMPALA-6125

After some chat with Tim, we talked about the possibilities of reducing the
dependence of boost. Since now we are using C++11, there are std::thread,
std::mutex, and std::condition_variable in the standard library.

On the other hand, the standard library likes to use exceptions in case of
errors, and AFAIK we don't like exceptions in Impala. Also, we already have
utility classes like ConditionVariable in the code base. In the kudu/util
directory, there is a Mutex class which is more conform to the Impala code
conventions than boost and the standard library. It also checks the
ownership of the mutex in debug mode, which can be useful for detecting
bugs.

Do you think it would be a good idea to create our own Mutex implementation
based on kudu/util/mutex?

BR,
Zoltan