Merge brlock.tdb and locking.tdb

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Merge brlock.tdb and locking.tdb

Samba - samba-technical mailing list
Hi!

I was just wondering whether there are any reasons that would prohibit merging
brlock.tdb and locking.tdb.

Unless I'm missing something, the record key is the same, we could add the brl data
to the struct share_mode_data IDL definition and update the brlock code to use a
different backing store.

This consolidation could be helpful in the implementation of a persistent
handles backing store.

Anyone?

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

Reply | Threaded
Open this post in threaded view
|

Re: Merge brlock.tdb and locking.tdb

Samba - samba-technical mailing list
On Sun, 29 Oct 2017 12:29:08 +0100, Ralph Böhme via samba-technical wrote:

> Unless I'm missing something, the record key is the same, we could add the brl data
> to the struct share_mode_data IDL definition and update the brlock code to use a
> different backing store.
>
> This consolidation could be helpful in the implementation of a persistent
> handles backing store.
>
> Anyone?

Sounds like a good idea to me. Would that also make removal of
DBWRAP_LOCK_ORDER a possibility?

Cheers, David

Reply | Threaded
Open this post in threaded view
|

Re: Merge brlock.tdb and locking.tdb

Samba - samba-technical mailing list
On Sun, Oct 29, 2017 at 06:25:54PM +0100, David Disseldorp via samba-technical wrote:

> On Sun, 29 Oct 2017 12:29:08 +0100, Ralph Böhme via samba-technical wrote:
>
> > Unless I'm missing something, the record key is the same, we could add the brl data
> > to the struct share_mode_data IDL definition and update the brlock code to use a
> > different backing store.
> >
> > This consolidation could be helpful in the implementation of a persistent
> > handles backing store.
> >
> > Anyone?
>
> Sounds like a good idea to me.

One argument for keeping brlock.tdb is the fact that we might have to
read it MUCH more often to implement mandatory byte range
locking. You don't want to parse a potentially large and complex
locking.tdb record for every read or write request.

> Would that also make removal of DBWRAP_LOCK_ORDER a possibility?

No, I don't think so. We have more than just those two, and we need to
keep the locking order correct.

Volker

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Merge brlock.tdb and locking.tdb

Samba - samba-technical mailing list
On Sun, Oct 29, 2017 at 08:39:02PM +0100, Volker Lendecke via samba-technical wrote:

> On Sun, Oct 29, 2017 at 06:25:54PM +0100, David Disseldorp via samba-technical wrote:
> > On Sun, 29 Oct 2017 12:29:08 +0100, Ralph Böhme via samba-technical wrote:
> >
> > > Unless I'm missing something, the record key is the same, we could add the brl data
> > > to the struct share_mode_data IDL definition and update the brlock code to use a
> > > different backing store.
> > >
> > > This consolidation could be helpful in the implementation of a persistent
> > > handles backing store.
> > >
> > > Anyone?
> >
> > Sounds like a good idea to me.
>
> One argument for keeping brlock.tdb is the fact that we might have to
> read it MUCH more often to implement mandatory byte range
> locking. You don't want to parse a potentially large and complex
> locking.tdb record for every read or write request.

Yep, that's the issue. That's why the byte range lock
data was originally kept separate from the open file
data.

Jeremy.

Reply | Threaded
Open this post in threaded view
|

Re: Merge brlock.tdb and locking.tdb

Samba - samba-technical mailing list
On Mon, Oct 30, 2017 at 09:14:47AM -0700, Jeremy Allison via samba-technical wrote:

> On Sun, Oct 29, 2017 at 08:39:02PM +0100, Volker Lendecke via samba-technical wrote:
> > On Sun, Oct 29, 2017 at 06:25:54PM +0100, David Disseldorp via samba-technical wrote:
> > > On Sun, 29 Oct 2017 12:29:08 +0100, Ralph Böhme via samba-technical wrote:
> > >
> > > > Unless I'm missing something, the record key is the same, we could add the brl data
> > > > to the struct share_mode_data IDL definition and update the brlock code to use a
> > > > different backing store.
> > > >
> > > > This consolidation could be helpful in the implementation of a persistent
> > > > handles backing store.
> > > >
> > > > Anyone?
> > >
> > > Sounds like a good idea to me.
> >
> > One argument for keeping brlock.tdb is the fact that we might have to
> > read it MUCH more often to implement mandatory byte range
> > locking. You don't want to parse a potentially large and complex
> > locking.tdb record for every read or write request.
>
> Yep, that's the issue. That's why the byte range lock
> data was originally kept separate from the open file
> data.

yeah, I was afraid that would be the reason. On the downside, by using seperate
dbs we have do the update-num-read-oplocks mumbo-jumbo, but I guess that's still
less overhead then unpacking a locking.tdb record for the mandatory brls. Hmpf.

Thanks!
-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

Reply | Threaded
Open this post in threaded view
|

Re: Merge brlock.tdb and locking.tdb

Samba - samba-technical mailing list
On Mon, Oct 30, 2017 at 07:19:21PM +0100, Ralph Böhme via samba-technical wrote:
> yeah, I was afraid that would be the reason. On the downside, by
> using seperate dbs we have do the update-num-read-oplocks
> mumbo-jumbo, but I guess that's still less overhead then unpacking a
> locking.tdb record for the mandatory brls. Hmpf.

Just brainstorming: We have the locking.tdb record cache now. Does
that help here? Maybe the real performance penalty for the
non-contended workload isn't as bad anymore as it used to be. Of
course, if a lot of byte range locking activity is going on, things
go down, but does that matter in real world cases?

Volker

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Merge brlock.tdb and locking.tdb

Samba - samba-technical mailing list
On Mon, Oct 30, 2017 at 09:06:44PM +0100, Volker Lendecke wrote:

> On Mon, Oct 30, 2017 at 07:19:21PM +0100, Ralph Böhme via samba-technical wrote:
> > yeah, I was afraid that would be the reason. On the downside, by
> > using seperate dbs we have do the update-num-read-oplocks
> > mumbo-jumbo, but I guess that's still less overhead then unpacking a
> > locking.tdb record for the mandatory brls. Hmpf.
>
> Just brainstorming: We have the locking.tdb record cache now. Does
> that help here? Maybe the real performance penalty for the
> non-contended workload isn't as bad anymore as it used to be. Of
> course, if a lot of byte range locking activity is going on, things
> go down, but does that matter in real world cases?

Only benchmarking/tests can tell :-).

Reply | Threaded
Open this post in threaded view
|

Re: Merge brlock.tdb and locking.tdb

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Mon, Oct 30, 2017 at 09:06:44PM +0100, Volker Lendecke wrote:

> On Mon, Oct 30, 2017 at 07:19:21PM +0100, Ralph Böhme via samba-technical wrote:
> > yeah, I was afraid that would be the reason. On the downside, by
> > using seperate dbs we have do the update-num-read-oplocks
> > mumbo-jumbo, but I guess that's still less overhead then unpacking a
> > locking.tdb record for the mandatory brls. Hmpf.
>
> Just brainstorming: We have the locking.tdb record cache now. Does
> that help here? Maybe the real performance penalty for the
> non-contended workload isn't as bad anymore as it used to be. Of
> course, if a lot of byte range locking activity is going on, things
> go down, but does that matter in real world cases?

Good point, thanks.

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

Reply | Threaded
Open this post in threaded view
|

Re: Merge brlock.tdb and locking.tdb

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Wed, Nov 01, 2017 at 10:45:41AM -0700, Jeremy Allison wrote:

> On Mon, Oct 30, 2017 at 09:06:44PM +0100, Volker Lendecke wrote:
> > On Mon, Oct 30, 2017 at 07:19:21PM +0100, Ralph Böhme via samba-technical wrote:
> > > yeah, I was afraid that would be the reason. On the downside, by
> > > using seperate dbs we have do the update-num-read-oplocks
> > > mumbo-jumbo, but I guess that's still less overhead then unpacking a
> > > locking.tdb record for the mandatory brls. Hmpf.
> >
> > Just brainstorming: We have the locking.tdb record cache now. Does
> > that help here? Maybe the real performance penalty for the
> > non-contended workload isn't as bad anymore as it used to be. Of
> > course, if a lot of byte range locking activity is going on, things
> > go down, but does that matter in real world cases?
>
> Only benchmarking/tests can tell :-).

indeed.

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/