Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Tom Lane]

Greg Smith  writes:
> The last time I tried to do this a few years ago I failed miserably and 
> never came back.  I know way more about building software now though, 
> and just got this to work for the first time.

BTW, another thing that should be in the try-try-again category is
seeing how close we could get to pgindent's results with GNU indent.
It seems clear to me that a process based on GNU indent would be a
lot easier for a lot of people.  We tried that once before, and couldn't
get close enough to want to consider switching, but maybe it just needs
a more determined effort and/or more recent versions of GNU indent.
(ISTR that we hit some things that seemed to be outright bugs in GNU
indent, but this was quite a few years ago.)

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Tom Lane]

Robert Haas  writes:
> ... Maybe someone out there is under the impression
> that I get high off of rejecting patches; but the statistics you cite
> from the CF app don't exactly support the contention that I'm going
> around looking for reasons to reject things, or if I am, I'm doing a
> pretty terrible job finding them.

Hm ... there are people out there who think *I* get high off rejecting
patches.  I have a t-shirt to prove it.  But I seem to be pretty
ineffective at it too, judging from these numbers.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Radosław Smogura]

Robert Haas  Monday 18 April 2011 03:06:17
> On Sun, Apr 17, 2011 at 5:32 PM, Radosław Smogura
> 
>  wrote:
> > Each process has simple "mirror" of shared descriptors.
> > 
> > I "believe" that modifications to buffer content may be only done when
> > holding exclusive lock (with some simple exceptions) (+ MVCC), actually
> > I saw only two things that can change already loaded data and cause
> > damage, you have described (setting hint bits during scan, and vacuum -
> > 1st may only cause, I think, that two processes will ask for same
> > transaction statuses , 2nd one is impossible as vacumm
> > requires exclusive pin). When buffer tag is changed the version of
> > buffer is bumped up, and checked against local version - this about
> > reading buffer.
> 
> Yes, an exclusive lock is required for substantive content changes.
> But if vacuum cleaning up the buffer is an issue for your patch, then
> it's probably also a problem if someone grabs an exclusive content
> lock and deletes the tuple (by setting XMAX) and some other backend
> later sees the old buffer contents after having in the meanwhile taken
> a new snapshot; or if likewise someone grabs an exclusive-lock, adds a
> tuple, and then your backend takes a new snapshot and then sees the
> old buffer contents.  Basically, any time someone grabs an
> exclusive-lock and releases it, it's necessary for all observers to
> see the updated contents by the time the exclusive lock is released.
> > In other cases after obtaining lock check is done if buffer has
> > associated updatable buffer and if local "mirror" has it too, then swap
> > should take place.
> 
> I think this check would have to be done every time someone
> share-locks the buffer, which seems rather expensive.
I don't treat as issues, but it's disadvantage.

> > Logic about updatable buffers is similar to "shared buffers", each
> > updatable buffer has pin count, and updatable buffer can't be free if
> > someone uses it, but in contrast to "normal buffers", updatable buffers
> > doesn't have any support for locking etc. Updatable buffers exists only
> > on free list, or when associated with buffer.
> 
> I don't see how you're going to get away with removing buffer locks.
> They exist for a reason, and adding mmap() to the mix is going to
> require MORE locking, not less.
> 
> > In future, I will change version to shared segment id, something like
> > relation's oid + block, but ids will have continuous numbering 1,2,3...,
> > so I will be able to bypass smgr/md during read, and tag version check -
> > this looks like faster solution.
> 
> I don't understand this part at all.
Versioning is witch approach where I thought about really often changes of 
mmaped areas, I allocated part of segments, but now the segment is mmaped with 
reservation, to it's full possible size, addresses of segments can't change 
(problem is only with segment deletion).

Regards,
Radek

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Tom Lane]

Andrew Dunstan  writes:
> On 04/17/2011 07:41 PM, Robert Haas wrote:
>>> That puts the overall patch acceptance rate at perhaps 75%.

>> That someone overstates the acceptance rate, because it ignores the
>> patches that people post and immediately get flamed to a well-done
>> crisp before adding them to the CF app, but there are not very many of
>> those any more.

> I don't believe there were ever terribly many of them.

Well, that number also ignores patches that were *committed* without
ever making it to the CF list.  There aren't terribly many of those
either I think, but it does happen, particularly for small patches.
If you want to argue about the acceptance rate for out-of-CF-process
patches you'd have to do some serious digging in the archives to say
anything about what it is.

But anyway this is quibbling.  The point I was trying to make is that
our patch acceptance rate is fairly far north of 50%, not south of it.
So we might hold people's feet to the fire a bit in the process, but
it's hardly impossible to get a patch committed.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Evaluation of secondary sort key.

[Jesper Krogh]

On 2011-04-09 18:54, Tom Lane wrote:

I think that would be a positive disimprovement.  The current design
guarantees that volatile sort expressions are evaluated exactly once,
in the order the rows are read from the data source.  There would be no
guarantees at all, either as to the number of evaluations or the order
in which they happen, if we tried to do evaluation only during the
actual sort.

If I could "only evaluate" the rows needed, then that would also
be a huge win, below case shifts what "definitely shouldn't be a seqscan"
into one due to a secondary sort key.


Another small problem is that any such thing would require carrying
along some kind of closure (ie, the expression and all its input
values), not just the final sort key value, in tuples being sorted.
The ensuing complexity, speed penalty, and increase in data volume
to be sorted would be paid by everybody, making this probably a net
performance loss when considered across all applications.

Getting the value for the first sortkey and carrying on a closure
for the rest would mostly (very often) be "optimal" ?

It would also enable a select that has to sortkeys to utilize an
index that only contains the primary sortkey, which is a huge
negative effect of what's being done today.

2011-04-18 07:12:43.931 testdb=# explain select id from testdb.testtable 
order by id limit 500;

 QUERY PLAN

 Limit  (cost=0.00..262.75 rows=500 width=4)
   ->  Index Scan using testtable_pkey on testtable  
(cost=0.00..15015567.84 rows=28573400 width=4)

(2 rows)

Time: 1.363 ms
2011-04-18 07:12:52.498 testdb=# explain select id from testdb.testtable 
order by id,name limit 500;

QUERY PLAN
---
 Limit  (cost=5165456.70..5165457.95 rows=500 width=35)
   ->  Sort  (cost=5165456.70..5236890.20 rows=28573400 width=35)
 Sort Key: id, name
 ->  Seq Scan on testtable  (cost=0.00..3741675.00 
rows=28573400 width=35)

(4 rows)

Time: 1.420 ms

Enabling any users to sort using multiple keys, without ending in Seq 
Scans somewhere down
the line seems to require multi dimension indexes on all combinations of 
colums


--
Jesper

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Greg Smith]

Andrew Dunstan wrote:
Now we could certainly make this quite a bit slicker. Apart from 
anything else, we should change the indent source code tarball so it 
unpacks into its own directory. Having it unpack into the current 
directory is ugly and unfriendly. And we should get rid of the "make 
clean" line in the install target of entab's makefile, which just 
seems totally ill-conceived.


I think the script I submitted upthread has most of the additional 
slickness needed here.  Looks like we both were working on documenting a 
reasonable way to do this at the same time the other day.  The idea of 
any program here relying on being able to write to /usr/local/bin as 
your example did makes this harder for people to run; that's why I made 
everything in the build tree and just pushed the appropriate directories 
into the PATH.


Since I see providing a script to automate this whole thing as the 
preferred way to make this easier, re-packaging the indent source 
tarball to extract to a directory doesn't seem worth the backwards 
compatibility trouble it will introduce.  Improving the entab makefile I 
don't have an opinion on.


It might also be worth setting it up so that instead of having to pass 
a path to a typedefs file on the command line, we default to a file 
sitting in, say, /usr/local/etc. Then you'd just be able to say 
"pgindent my_file.c".


OK, so I need to update my script to handle either indenting a single 
file, or doing all of them.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Robert Haas]

On Sun, Apr 17, 2011 at 5:32 PM, Radosław Smogura
 wrote:
> Each process has simple "mirror" of shared descriptors.
>
> I "believe" that modifications to buffer content may be only done when holding
> exclusive lock (with some simple exceptions) (+ MVCC), actually I saw only two
> things that can change already loaded data and cause damage, you have
> described (setting hint bits during scan, and vacuum - 1st may only cause, I
> think, that two processes will ask for same transaction statuses  vacuum>, 2nd one is impossible as vacumm requires exclusive pin). When buffer
> tag is changed the version of buffer is bumped up, and checked against local
> version - this about reading buffer.

Yes, an exclusive lock is required for substantive content changes.
But if vacuum cleaning up the buffer is an issue for your patch, then
it's probably also a problem if someone grabs an exclusive content
lock and deletes the tuple (by setting XMAX) and some other backend
later sees the old buffer contents after having in the meanwhile taken
a new snapshot; or if likewise someone grabs an exclusive-lock, adds a
tuple, and then your backend takes a new snapshot and then sees the
old buffer contents.  Basically, any time someone grabs an
exclusive-lock and releases it, it's necessary for all observers to
see the updated contents by the time the exclusive lock is released.

> In other cases after obtaining lock check is done if buffer has associated
> updatable buffer and if local "mirror" has it too, then swap should take
> place.

I think this check would have to be done every time someone
share-locks the buffer, which seems rather expensive.

> Logic about updatable buffers is similar to "shared buffers", each updatable
> buffer has pin count, and updatable buffer can't be free if someone uses it,
> but in contrast to "normal buffers", updatable buffers doesn't have any
> support for locking etc. Updatable buffers exists only on free list, or when
> associated with buffer.

I don't see how you're going to get away with removing buffer locks.
They exist for a reason, and adding mmap() to the mix is going to
require MORE locking, not less.

> In future, I will change version to shared segment id, something like
> relation's oid + block, but ids will have continuous numbering 1,2,3..., so I
> will be able to bypass smgr/md during read, and tag version check - this looks
> like faster solution.

I don't understand this part at all.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Andrew Dunstan]

On 04/17/2011 07:41 PM, Robert Haas wrote:



That puts the overall patch acceptance rate at perhaps 75%.

That someone overstates the acceptance rate, because it ignores the
patches that people post and immediately get flamed to a well-done
crisp before adding them to the CF app, but there are not very many of
those any more.



I don't believe there were ever terribly many of them.

cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Robert Haas]

On Sun, Apr 17, 2011 at 7:13 PM, Tom Lane  wrote:
> Dan Ports  writes:
>> ... But I am aware of other cases in which people in the academic community
>> have done work that could well be of interest to the Postgres community
>> but didn't submit their work here. In part, that was because they did
>> not have the time/motivation to get the work into a polished,
>> acceptable state, and in part because of the reputation of the
>> community.
>
> Well, if the author isn't interested in getting the work into a
> committable state, it's not clear what's the point of submitting it.
> It's not like people who are eager to do that kind of work on someone
> else's patch are thick on the ground.
>
> But I think the perception that we reject most patches is misplaced.
> It's fairly easy to demonstrate that the default assumption around here
> is that submitted patches will get committed.  Looking at the past five
> commitfests (covering a bit more than a year), we committed 201 out of
> 305 patches, and only 10 were actually marked "rejected".  I'm too lazy
> to try to determine just which of the 94 returned-with-feedback patches
> got committed in later fests, but a quick scan suggests at least 20 did,
> and there are more that might get committed in the next fest.  That puts
> the overall patch acceptance rate at perhaps 75%.

That someone overstates the acceptance rate, because it ignores the
patches that people post and immediately get flamed to a well-done
crisp before adding them to the CF app, but there are not very many of
those any more.  (If someone thinks I'm wrong about this, they are
cheerfully invited to provide the evidence.  It is certainly possible
that I'm guilty of selective memory; this is just how I remember it.)

> At least since the CF
> mechanism was instituted, it seems to me that the dynamic has been that
> someone who doesn't like a patch has to show cause why it shouldn't get
> committed, not the other way around.  Robert's recent comment that he
> was afraid he'd have to spend time digging into the mmap patch to prove
> it was broken reflects exactly that feeling.

Yes, and I think it's also telling that the response to that was not
"oh, gee, if Robert thinks this patch is totally busted, we'd better
take that concern seriously" but rather "stop picking on the guy who
submitted the patch".  Maybe someone out there is under the impression
that I get high off of rejecting patches; but the statistics you cite
from the CF app don't exactly support the contention that I'm going
around looking for reasons to reject things, or if I am, I'm doing a
pretty terrible job finding them.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Tom Lane]

Dan Ports  writes:
> ... But I am aware of other cases in which people in the academic community
> have done work that could well be of interest to the Postgres community
> but didn't submit their work here. In part, that was because they did
> not have the time/motivation to get the work into a polished,
> acceptable state, and in part because of the reputation of the
> community.

Well, if the author isn't interested in getting the work into a
committable state, it's not clear what's the point of submitting it.
It's not like people who are eager to do that kind of work on someone
else's patch are thick on the ground.

But I think the perception that we reject most patches is misplaced.
It's fairly easy to demonstrate that the default assumption around here
is that submitted patches will get committed.  Looking at the past five
commitfests (covering a bit more than a year), we committed 201 out of
305 patches, and only 10 were actually marked "rejected".  I'm too lazy
to try to determine just which of the 94 returned-with-feedback patches
got committed in later fests, but a quick scan suggests at least 20 did,
and there are more that might get committed in the next fest.  That puts
the overall patch acceptance rate at perhaps 75%.  At least since the CF
mechanism was instituted, it seems to me that the dynamic has been that
someone who doesn't like a patch has to show cause why it shouldn't get
committed, not the other way around.  Robert's recent comment that he
was afraid he'd have to spend time digging into the mmap patch to prove
it was broken reflects exactly that feeling.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Dan Ports]

On Sat, Apr 16, 2011 at 11:26:34PM -0500, Kevin Grittner wrote:
> I have to say, I've been rather mystified by the difficulty
> attributed to running pgindent.  During work on the SSI patch, I ran
> it about once every two weeks on files involved in the patch

Well, as a counterpoint: during work on the SSI patch, I did *not* run
pgindent. I attempted to, at one point, but was discouraged when I
realized that it required BSD indent and my Linux machine only had GNU
indent. That meant I would need to find, build, and install a new
version of indent, and keep it separate from my existing GNU indent.
Hardly impossible, but it's a lot more of a hassle than simply running a
script, and it left me wondering if I was going to run into other
issues even if I did get the right indent installed.

Andrew's instructions upthread would certainly have been helpful to
have in the pgindent README.

(To be fair, I would probably have made much more of an effort to run
pgindent if I didn't already know Kevin was running it periodically on
the SSI code.)


> And I can't help but wonder why, in an off-list discussion with
> Michael Cahill about the SSI technology he commented that he was
> originally intending to implement the technique in PostgreSQL, but
> later chose Oracle Berkeley DB and then latter InnoDB instead. 
> *Maybe* he was looking toward being hired by Oracle, and *maybe* it
> was because the other databases already had predicate locking and
> true serializable transaction isolation levels -- but was part of it
> the reputation of the community?  I keep wondering.

I would discount the first explanation (being hired at Oracle)
entirely. I think the second explanation is the correct one: it's
simply much more difficult to implement SSI atop a database that does
not already have predicate locking (as we know!)

But I am aware of other cases in which people in the academic community
have done work that could well be of interest to the Postgres community
but didn't submit their work here. In part, that was because they did
not have the time/motivation to get the work into a polished,
acceptable state, and in part because of the reputation of the
community. 

Dan

-- 
Dan R. K. Ports  MIT CSAILhttp://drkp.net/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Radosław Smogura]

Robert Haas  Sunday 17 April 2011 22:01:55
> On Sun, Apr 17, 2011 at 11:48 AM, Tom Lane  wrote:
> > =?utf-8?q?Rados=C5=82aw_Smogura?=  writes:
> >> Tom Lane  Sunday 17 April 2011 01:35:45
> >> 
> >>> ... Huh?  Are you saying that you ask the kernel to map each individual
> >>> shared buffer separately?  I can't believe that's going to scale to
> >>> realistic applications.
> >> 
> >> No, I do
> >> mrempa(mmap_buff_A, MAP_FIXED, temp);
> >> mremap(shared_buff_Y, MAP_FIXED, mmap_buff_A),
> >> mrempa(tmp, MAP_FIXED, mmap_buff_A).
> > 
> > There's no mremap() in the Single Unix Spec, nor on my ancient HPUX box,
> > nor on my quite-up-to-date OS X box.  The Linux man page for it says
> > "This call is Linux-specific, and should not be used in programs
> > intended to be portable."  So if the patch is dependent on that call,
> > it's dead on arrival from a portability standpoint.
> > 
> > But in any case, you didn't explain how use of mremap() avoids the
> > problem of the kernel having to maintain a separate page-mapping-table
> > entry for each individual buffer.  (Per process, yet.)  If that's what's
> > happening, it's going to be a significant performance penalty as well as
> > (I suspect) a serious constraint on how many buffers can be managed.
> 
> I share your suspicions, although no harm in measuring it.
> 
> But I don't understand is how this approach avoids the problem of
> different processes seeing different buffer contents.  If backend A
> has the buffer mmap'd and backend B wants to modify it (and changes
> the mapping), backend A is still looking at the old buffer contents,
> isn't it?  And then things go boom.

Each process has simple "mirror" of shared descriptors.

I "believe" that modifications to buffer content may be only done when holding 
exclusive lock (with some simple exceptions) (+ MVCC), actually I saw only two 
things that can change already loaded data and cause damage, you have 
described (setting hint bits during scan, and vacuum - 1st may only cause, I 
think, that two processes will ask for same transaction statuses , 2nd one is impossible as vacumm requires exclusive pin). When buffer 
tag is changed the version of buffer is bumped up, and checked against local 
version - this about reading buffer. 

In other cases after obtaining lock check is done if buffer has associated 
updatable buffer and if local "mirror" has it too, then swap should take 
place.

Logic about updatable buffers is similar to "shared buffers", each updatable 
buffer has pin count, and updatable buffer can't be free if someone uses it, 
but in contrast to "normal buffers", updatable buffers doesn't have any 
support for locking etc. Updatable buffers exists only on free list, or when 
associated with buffer.

In future, I will change version to shared segment id, something like 
relation's oid + block, but ids will have continuous numbering 1,2,3..., so I 
will be able to bypass smgr/md during read, and tag version check - this looks 
like faster solution.

Regards,
Radek

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Robert Haas]

On Sun, Apr 17, 2011 at 12:26 AM, Kevin Grittner
 wrote:
> Now, the other aspect to this whole discussion is that people often
> have code they have developed for academic purposes or for their own
> use which they want to offer to the community "FWIW", and I think we
> sometimes miss an opportunity to take advantage of someone else's
> work because of an assumption that they have some vested interest in
> it's acceptance.  The fact that someone doesn't care enough to try to
> work with the community to get their patch accepted doesn't *always*
> mean that we're better off for ignoring that patch.  Maybe that's
> true 90% of the time or better, but it seems to me that sometimes our
> community is a bit provincial.

We are.

On the other hand, cleaning up other people's not-ready-for-prime-time
patches isn't free.  If I spend 4 hours cleaning up a patch in
preparation for a commit, then that's 4 hours I don't get to spend on
my own work.  And since I *already* spend 3 or 4 times as much energy
on other people's work as I do on my own, I'm not willing to go much
further in that direction; if anything, I think I'd like to roll it
back a bit.  On the other hand, I am emphatically in favor of other
people who are not me being willing to do that kind of work; I think
it benefits our whole community, much as the work of people who write
their own patches or review or volunteer in any other way benefits our
whole community.

Because I commit approximately 10 patches per CommitFest, and review
perhaps another 5-10 that I don't end up committing (either because
they get rejected or because someone else commits them), the amount of
time that I can afford to spend on each of those patches is limited.
Generally, if I can't commit a normal-size patch in half an hour of
looking at it, I send back a review and move on.  For some patches
that I particularly care about, I have on occasion invested as much as
2-3 days (most recently, a big chunk of my Christmas vacation) to get
them beaten into shape for a commit.  I'd be happy to devote more time
per patch, but it ain't gonna happen as long as the number that I have
to handle to get the CommitFest finished on time remains in the
two-digit range.

That having been said, the kind of fixing up that you're talking about
*does* happen, when someone cares enough to make it happen.  We have
numerous examples in the archives where person A submits a patch, and
person B reviews it and, in lieu of a review, posts an updated patch,
sometimes when person A has meanwhile totally disappeared, or when
they haven't completely disappeared but don't have time to work on it.
 This is actually quite commonplace; it just doesn't happen for every
patch.  It tends to happen only for the things someone is really
excited about because, well, fixing up someone else's bad code is not
one of life's great pleasures.  It'd be nice if we had even more of it
than we do, but this is an all-volunteer organization.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Andres Freund]

On Sunday 17 April 2011 22:09:24 Radosław Smogura wrote:
> I only know Phenom has 4096 entries I think and this covers 16MB of
> memory. 
The numbers I cited where intels before and after core2.

Andres

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Radosław Smogura]

Andres Freund  Sunday 17 April 2011 20:02:11
> On Sunday 17 April 2011 19:26:31 Radosław Smogura wrote:
> > Kernel merges vm_structs. So mappings are compacted. I'm not kernel
> > specialist, but skipping memory consumption, for not compacted mappings,
> > kernel uses btrees for dealing with  TLB, so it should not matter if
> > there is  100 vm_structs or 10 vm_structs.
> 
> But the CPUs TLB cache has maybe 16/256 (1lvl, 2nd) to 64/512 entries. That
> will mean that there will be cachemisses all over.
> Additionally your scheme requires flushing it regularly...
> 
> Andres

I only know Phenom has 4096 entries I think and this covers 16MB of memory. 
But I was taking about memory usage of struct vm_struct in kernel. I tries as 
well with huge pages, but I can't write really fast allocator for this, it's 
slower then malloc, maybe from different reasons.

Regards,
Radek

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] ALTER TABLE INHERIT vs collations

[Tom Lane]

Peter Eisentraut  writes:
> On Sat, 2011-04-16 at 18:23 -0400, Tom Lane wrote:
>> Right at the moment, ALTER INHERIT doesn't verify that collations
>> match in a proposed inheritance child.

> It should be prevented, I think.

BTW, on looking through the source to find the cause, I see that the
reason it's not prevented is that MergeAttributesIntoExisting() failed
to check for collation match.  But the other three places in tablecmds.c
that check for child column type matches do check collation too.  So it
seems clear that this was just an oversight in one case.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Robert Haas]

On Sun, Apr 17, 2011 at 11:48 AM, Tom Lane  wrote:
> =?utf-8?q?Rados=C5=82aw_Smogura?=  writes:
>> Tom Lane  Sunday 17 April 2011 01:35:45
>>> ... Huh?  Are you saying that you ask the kernel to map each individual
>>> shared buffer separately?  I can't believe that's going to scale to
>>> realistic applications.
>
>> No, I do
>> mrempa(mmap_buff_A, MAP_FIXED, temp);
>> mremap(shared_buff_Y, MAP_FIXED, mmap_buff_A),
>> mrempa(tmp, MAP_FIXED, mmap_buff_A).
>
> There's no mremap() in the Single Unix Spec, nor on my ancient HPUX box,
> nor on my quite-up-to-date OS X box.  The Linux man page for it says
> "This call is Linux-specific, and should not be used in programs
> intended to be portable."  So if the patch is dependent on that call,
> it's dead on arrival from a portability standpoint.
>
> But in any case, you didn't explain how use of mremap() avoids the
> problem of the kernel having to maintain a separate page-mapping-table
> entry for each individual buffer.  (Per process, yet.)  If that's what's
> happening, it's going to be a significant performance penalty as well as
> (I suspect) a serious constraint on how many buffers can be managed.

I share your suspicions, although no harm in measuring it.

But I don't understand is how this approach avoids the problem of
different processes seeing different buffer contents.  If backend A
has the buffer mmap'd and backend B wants to modify it (and changes
the mapping), backend A is still looking at the old buffer contents,
isn't it?  And then things go boom.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Tom Lane]

Joshua Berkus  writes:
> I, for one, am glad he did this work.  We've discussed MMAP in the code off 
> and on for years, but nobody wanted to do the work to test it.  Now someone 
> has, and we can decide whether it's worth pursuing based on the numbers.

Well, the troubling issue is that it's not clear whether this patch is
realistic enough to think that performance measurements based on it
are representative of the whole idea of using mmap.  The business of
remapping individual buffers in order to transition them to writable
state seems likely to me to be a huge performance penalty --- first
there's the direct cost of having to incur a kernel call each time we
do that, and second there's the distributed cost of asking the kernel
to manage thousands or millions of tiny mappings.

IOW, if this patch shows little or no performance improvement (as seems
likely to happen at scale), that doesn't prove that mmap in general
isn't potentially interesting, only that this isn't the right way to
approach it.

Still, if you do some tests and don't find a win, that might save time
compared to actually trying to understand and vet the patch ...

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Joshua Berkus]

Robert,

> Actually, I'd walk through fire for a 10% performance improvement if
> it meant only a *risk* to stability.

Depends on the degree of risk.  MMAP has the potential to introduce instability 
into areas of the code which have been completely reliable for years.  Adding 
20 new coredump cases with data loss for a 10% improvement seems like a poor 
bargain to me.  It doesn't help that the only DB to rely heavily on MMAP 
(MongoDB) is OSSDB's paragon of data loss.

However, in the case where the database is larger than RAM ... or better, 90% 
of RAM ... MMAP has the theoretical potential to improve performance quite a 
bit more than 10% ... try up to 900% on some queries.  However, I'd like to 
prove that in a test before we bother even debating the fundamental obstacles 
to using MMAP.  It's possible that these theoretical performance benefits will 
not materialize, even without data safeguards.
 
> The problem is that this is
> likely unfixably broken. In particular, I think the first sentence of
> Tom's response hit it right on the nose, and mirrors my own thoughts
> on the subject. To have any chance of working, you'd need to track
> buffer pins and shared/exclusive content locks for the pages that were
> being accessed outside of shared buffers; otherwise someone might be
> looking at a stale copy of the page.

Nothing is unfixable.  The question is whether it's worth the cost.  Let me see 
if I can build a tree with Radislaw's patch, and do some real performance tests.

I, for one, am glad he did this work.  We've discussed MMAP in the code off and 
on for years, but nobody wanted to do the work to test it.  Now someone has, 
and we can decide whether it's worth pursuing based on the numbers.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com
San Francisco

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Andres Freund]

On Sunday 17 April 2011 19:26:31 Radosław Smogura wrote:
> Kernel merges vm_structs. So mappings are compacted. I'm not kernel 
> specialist, but skipping memory consumption, for not compacted mappings, 
> kernel uses btrees for dealing with  TLB, so it should not matter if there
> is  100 vm_structs or 10 vm_structs.
But the CPUs TLB cache has maybe 16/256 (1lvl, 2nd) to 64/512 entries. That 
will mean that there will be cachemisses all over.
Additionally your scheme requires flushing it regularly...

Andres

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Radosław Smogura]

Tom Lane  Sunday 17 April 2011 17:48:56
> =?utf-8?q?Rados=C5=82aw_Smogura?=  writes:
> > Tom Lane  Sunday 17 April 2011 01:35:45
> > 
> >> ... Huh?  Are you saying that you ask the kernel to map each individual
> >> shared buffer separately?  I can't believe that's going to scale to
> >> realistic applications.
> > 
> > No, I do
> > mrempa(mmap_buff_A, MAP_FIXED, temp);
> > mremap(shared_buff_Y, MAP_FIXED, mmap_buff_A),
> > mrempa(tmp, MAP_FIXED, mmap_buff_A).
> 
> There's no mremap() in the Single Unix Spec, nor on my ancient HPUX box,
> nor on my quite-up-to-date OS X box.  The Linux man page for it says
> "This call is Linux-specific, and should not be used in programs
> intended to be portable."  So if the patch is dependent on that call,
> it's dead on arrival from a portability standpoint.
Good point. This is from initial concept, and actually I done this to do not 
leave gaps in VM in which library or something could be mmaped. Last time I 
think about using mmap to replace just one VM page.

> But in any case, you didn't explain how use of mremap() avoids the
> problem of the kernel having to maintain a separate page-mapping-table
> entry for each individual buffer.  (Per process, yet.)  If that's what's
> happening, it's going to be a significant performance penalty as well as
> (I suspect) a serious constraint on how many buffers can be managed.
> 
>   regards, tom lane
Kernel merges vm_structs. So mappings are compacted. I'm not kernel 
specialist, but skipping memory consumption, for not compacted mappings, 
kernel uses btrees for dealing with  TLB, so it should not matter if there is 
100 vm_structs or 10 vm_structs.

Swap isn't made everywhere. When buffer is initialy read (privaterefcount 
==1), then any access to this buffer will directly point to latest valid area. 
If it has assigned shmem area then this will be used. I plan to add 
"readbuffer for update" to prevent swaps, when it's almost sure that buffer 
will be used for update.

I measured performance of page modifications (with unpining, full process on 
stand alone unit test) it's 2x-3x more time of normal page reads, but this 
result may not be sure, as I saw memcpy to memory above 2GB is slower then 
memcpy to first 2GB (this may be idea to try to put some shared structs < 
2GB).

I know that this patch is big question. Sometimes I'm optimistic, and 
sometimes I'm pessimistic about final result.

Regards,
Radek

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] blah blah set client_encoding segfault

[Tom Lane]

I wrote:
> Not for me ...

Oh ... the *second* time, it fails for me.  Doh.  Will fix.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] blah blah set client_encoding segfault

[Tom Lane]

Peter Eisentraut  writes:
> With the latest master branch:

> initdb -D somewhere --locale=C --encoding=LATIN1

> Then in psql:

> set client_encoding to utf8;

> causes a segfault in the backend.

Not for me ...

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

The big picture for patch submission (was Re: [HACKERS] MMAP Buffers)

[Tom Lane]

Robert Haas  writes:
> ... But please, everyone feel free to continue bashing me for
> wanting a readable patch with some understandable submission notes.

What he said.  All this obsessing over whether the mmap patch could or
should have been run through pgindent is missing the big picture.
Namely, that no design documentation or theory-of-operation was offered,
and people are trying to extract that information from the code, and
it's just too messy for that to be feasible.  (The patch isn't really
short of comments, but half of the comments seem to be TODOs or author's
questions to himself about whether something will work, and so they just
aren't particularly helpful to someone trying to understand what the
patch does or whether it will work.)

I think that rather than complaining about formatting, we should be
complaining about not following the overall patch submission process
and not providing adequate documentation.  Most of the questions that
people are asking right now could have been answered on the strength of
a design sketch, before any code had been written at all.  For a patch
as complicated and invasive as this, there should be a design sketch,
which perhaps gets fleshed out into a README file in the final patch.

The Submitting_a_Patch wiki page does touch on the point of getting some
early design feedback before you even try to write a patch, but I think
it could do with more emphasis on the issue.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Tom Lane]

=?utf-8?q?Rados=C5=82aw_Smogura?=  writes:
> Tom Lane  Sunday 17 April 2011 01:35:45
>> ... Huh?  Are you saying that you ask the kernel to map each individual
>> shared buffer separately?  I can't believe that's going to scale to
>> realistic applications.

> No, I do 
> mrempa(mmap_buff_A, MAP_FIXED, temp); 
> mremap(shared_buff_Y, MAP_FIXED, mmap_buff_A), 
> mrempa(tmp, MAP_FIXED, mmap_buff_A). 

There's no mremap() in the Single Unix Spec, nor on my ancient HPUX box,
nor on my quite-up-to-date OS X box.  The Linux man page for it says
"This call is Linux-specific, and should not be used in programs
intended to be portable."  So if the patch is dependent on that call,
it's dead on arrival from a portability standpoint.

But in any case, you didn't explain how use of mremap() avoids the
problem of the kernel having to maintain a separate page-mapping-table
entry for each individual buffer.  (Per process, yet.)  If that's what's
happening, it's going to be a significant performance penalty as well as
(I suspect) a serious constraint on how many buffers can be managed.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Andrew Dunstan]

On 04/17/2011 09:51 AM, Andrew Dunstan wrote:



On 04/17/2011 04:26 AM, Andrew Dunstan wrote:



Now we could certainly make this quite a bit slicker. Apart from 
anything else, we should change the indent source code tarball so it 
unpacks into its own directory. Having it unpack into the current 
directory is ugly and unfriendly. And we should get rid of the "make 
clean" line in the install target of entab's makefile, which just 
seems totally ill-conceived.


It might also be worth setting it up so that instead of having to 
pass a path to a typedefs file on the command line, we default to a 
file sitting in, say, /usr/local/etc. Then you'd just be able to say 
"pgindent my_file.c".





OK, I have most of these bits.

A new tarball of indent is available at
 
and if everyone agrees I'll push it out to the mirrors.


Attached are two patches, one to remove some infelicity in the entab 
makefile, and the other to allow skipping specifying the typedefs file 
location either by setting it in an environment variable or by putting 
it in a hard coded location.





... and this one has a typo fixed.

cheers

andrew


diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 05f69ef..02f2b93 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -13,13 +13,32 @@
 #
 #	void x(struct xxc * a);
 
-if [ "$#" -lt 2 ]
-then	echo "Usage:  $(basename $0) typedefs file [...]" 1>&2
-	exit 1
+# look for typedefs in the first argument, if there is a second argument and
+# the first argument contains the string 'typedef' in its name, or in the
+# environment setting PGTYPEDEFS, or in a hardcoded location, whichever
+# matches first.
+
+
+if [ $# -gt 1 ]
+then
+	case `basename $0` in
+		*typedef*) 
+			TYPDEFS=$1
+			shift
+			;;
+		*)
+			;;
+	esac
 fi
 
-TYPEDEFS="$1"
-shift
+test -z "$TYPEDEFS" && TYPEDEFS=$PGTYPEDEFS
+test -z "$TYPEDEFS" && TYPEDEFS=/usr/local/etc/pgtypedefs.list
+
+if [ ! -f "$TYPEDEFS" ]
+then
+	echo "Cannot find typedefs file '$TYPEDEFS'"
+	exit 1
+fi 
 
 if [ -z "$INDENT" ]
 then

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Andrew Dunstan]

On 04/17/2011 04:26 AM, Andrew Dunstan wrote:



Now we could certainly make this quite a bit slicker. Apart from 
anything else, we should change the indent source code tarball so it 
unpacks into its own directory. Having it unpack into the current 
directory is ugly and unfriendly. And we should get rid of the "make 
clean" line in the install target of entab's makefile, which just 
seems totally ill-conceived.


It might also be worth setting it up so that instead of having to pass 
a path to a typedefs file on the command line, we default to a file 
sitting in, say, /usr/local/etc. Then you'd just be able to say 
"pgindent my_file.c".





OK, I have most of these bits.

A new tarball of indent is available at
 and 
if everyone agrees I'll push it out to the mirrors.


Attached are two patches, one to remove some infelicity in the entab 
makefile, and the other to allow skipping specifying the typedefs file 
location either by setting it in an environment variable or by putting 
it in a hard coded location.


cheers

andrew

diff --git a/src/tools/entab/Makefile b/src/tools/entab/Makefile
index de81818..6372971 100644
--- a/src/tools/entab/Makefile
+++ b/src/tools/entab/Makefile
@@ -20,9 +20,7 @@ halt.o	: halt.c
 clean:
 	rm -f *.o $(TARGET) log core
 
-install:
-	make clean
-	make CFLAGS=-O
+install: $(TARGET)
 	install -s $(TARGET) $(BINDIR)
 	rm -f $(BINDIR)/detab
 	ln $(BINDIR)/$(TARGET) $(BINDIR)/detab
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 05f69ef..08dde0c 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -13,13 +13,32 @@
 #
 #	void x(struct xxc * a);
 
-if [ "$#" -lt 2 ]
-then	echo "Usage:  $(basename $0) typedefs file [...]" 1>&2
-	exit 1
+# look for typedefs in the first argument, if there is a second argument and
+# the first argument contains the string 'typedef' in its name, or in the
+# environment setting PGTYPEDEFS, or in a hardcoded location, whichever
+# matches first.
+
+
+if [ $# -gt 1 ]
+then
+	case `basenname $0` in
+		*typedef*) 
+			TYPDEFS=$1
+			shift
+			;;
+		*)
+			;;
+	esac
 fi
 
-TYPEDEFS="$1"
-shift
+test -z "$TYPEDEFS" && TYPEDEFS=$PGTYPEDEFS
+test -z "$TYPEDEFS" && TYPEDEFS=/usr/local/etc/pgtypedefs.list
+
+if [ ! -f "$TYPEDEFS" ]
+then
+	echo "Cannot find typedefs file '$TYPEDEFS'"
+	exit 1
+fi 
 
 if [ -z "$INDENT" ]
 then

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] blah blah set client_encoding segfault

[Peter Eisentraut]

[sent this a few days ago, must have gotten lost in "moderation" because
of the original subject]

With the latest master branch:

initdb -D somewhere --locale=C --encoding=LATIN1

Then in psql:

set client_encoding to utf8;

causes a segfault in the backend.  The backtrace:

#0  SetClientEncoding (encoding=6) at mbutils.c:227
#1  0x081de584 in assign_client_encoding (newval=0x90f3a70 "UTF8", 
extra=0x90f3980) at variable.c:822
#2  0x083af296 in set_config_option (name=0x916f640 "client_encoding", 
value=0x9193d9c "utf8", context=PGC_SUSET, source=PGC_S_SESSION, 
action=GUC_ACTION_SET, changeVal=1 '\001') at guc.c:5633
#3  0x083b0f4b in ExecSetVariableStmt (stmt=0x916f6bc) at guc.c:6066
#4  0x082cf758 in PortalRunUtility (portal=0x9191d94, utilityStmt=0x916f6bc, 
isTopLevel=1 '\001', dest=0x916f890, 
completionTag=0xff89e2f4 "") at pquery.c:1184
#5  0x082d02de in PortalRunMulti (portal=0x9191d94, isTopLevel=1 '\001', 
dest=0x916f890, altdest=0x916f890, completionTag=0xff89e2f4 "")
at pquery.c:1322
#6  0x082d11b4 in PortalRun (portal=0x9191d94, count=2147483647, isTopLevel=1 
'\001', dest=0x916f890, altdest=0x916f890, 
completionTag=0xff89e2f4 "") at pquery.c:813
#7  0x082ccb45 in exec_simple_query (argc=2, argv=0x90f4410, username=0x90f4258 
"peter") at postgres.c:1018
#8  PostgresMain (argc=2, argv=0x90f4410, username=0x90f4258 "peter") at 
postgres.c:3919
#9  0x08282c1e in BackendRun (port=0x9110670) at postmaster.c:3590
#10 BackendStartup (port=0x9110670) at postmaster.c:3275
#11 0x0828331a in ServerLoop () at postmaster.c:1449
#12 0x08283e59 in PostmasterMain (argc=3, argv=0x90f2f60) at postmaster.c:1110
#13 0x0821bae0 in main (argc=3, argv=0x90f2f60) at main.c:199

Looks like the conversion table is accessed before it's initialized.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] ALTER TABLE INHERIT vs collations

[Peter Eisentraut]

On Sat, 2011-04-16 at 21:52 -0400, Jean-Pierre Pelletier wrote:
> One use case for this might be with constraint exclusion where there
> would be one
> partition per collation (language) with queries against the parent table 
> always
> being for exactly one language.

If you really wanted that, you can always use a view to make it
explicit.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] ALTER TABLE INHERIT vs collations

[Peter Eisentraut]

On Sat, 2011-04-16 at 18:23 -0400, Tom Lane wrote:
> Right at the moment, ALTER INHERIT doesn't verify that collations
> match in a proposed inheritance child.

It should be prevented, I think.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Andrew Dunstan]

On 04/17/2011 04:08 AM, Greg Smith wrote:

Andrew Dunstan wrote:
What makes you think this isn't possible to run pgindent? There are 
no secret incantations.


The first hit newbies find looking for info about pgident is 
http://blog.hagander.net/archives/185-pgindent-vs-dash.html which sure 
looks like secret incantations to me.  The documentation 
src/tools/pgindent/README reads like a magic spell too:


   find . -name '*.[ch]' -type f -print | \
   egrep -v -f src/tools/pgindent/exclude_file_patterns | \
   xargs -n100 pgindent src/tools/pgindent/typedefs.list



That's the incantation for indenting the whole of the source code. But 
very few people want to do that. Most people just want to indent a 
single file, for which the incantation is "pgindent path_to_typedefs 
my_file.c". See in another message my suggestion for defaulting the 
typedefs arg, so you'd just be able to say "pg_indent my_file.c".



cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Andrew Dunstan]

On 04/17/2011 02:16 AM, Jeff Janes wrote:

On 4/16/11, Andrew Dunstan  wrote:


What makes you think this isn't possible to run pgindent? There are no
secret incantations.

A while ago I spent a few hours trying to run it and gave up.  I think
it was something about needing some obscure BSD version of some tool
which conflicted with just about everything else on the system.  I can
try again and report back if anyone cares.



A few hours? Seriously?

Here's what I just did, starting from scratch. It took me a few minutes.

   * wget ftp://ftp.postgresql.org/pub/dev/indent.netbsd.patched.tgz
   * mkdir bsdindent && cd bdsindent && tar -z -xf
 .../indent.netbsd.patched.tgz
   * make
   * mv indent indent_for_pg
   * sudo install -s -o bin -g bin indent_for_pg /usr/local/bin
   * cd ../pg_head/src/tools/entab
   * sudo make install
   * cd ../pgindent
   * sed -i 's/INDENT=indent/INDENT=indent_for_pg/' pgindent
   * sudo  install -s -o bin -g bin pgindent /usr/local/bin


Now we could certainly make this quite a bit slicker. Apart from 
anything else, we should change the indent source code tarball so it 
unpacks into its own directory. Having it unpack into the current 
directory is ugly and unfriendly. And we should get rid of the "make 
clean" line in the install target of entab's makefile, which just seems 
totally ill-conceived.


It might also be worth setting it up so that instead of having to pass a 
path to a typedefs file on the command line, we default to a file 
sitting in, say, /usr/local/etc. Then you'd just be able to say 
"pgindent my_file.c".


But it shouldn't take anyone hours to set up.

cheers

andrew





--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Greg Smith]

Robert Haas wrote:

But it turns out that it doesn't really matter.  Whitespace or no
whitespace, if you don't read the diff before you hit send, it's
likely to contain some irrelevant cruft, whether whitespace changes or
otherwise.
  


Right.  Presuming that pgident will actually solve anything leaps over 
two normally incorrect assumptions:


-That the main tree was already formatted with pgident before you 
started, so no stray diffs will result from it touching things the 
submitter isn't even involved in.


-There is no larger code formatting or diff issues except for spacing.

This has been a nagging loose end for a while, so I'd like to see 
pgindent's rough edges get sorted out so it's easier to use.  But 
whitespace errors because of bad editors are normally just a likely sign 
of a patch with bigger problems, rather than something that can get 
fixed and then submissions is good.  There is no substitute for the 
discipline of reading your own diff before submission.  I'll easily 
obsess over mine for an hour before I submit something major, and that 
time is always well spent.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] MMAP Buffers

[Radosław Smogura]

Tom Lane  Sunday 17 April 2011 01:35:45
> =?utf-8?q?Rados=C5=82aw_Smogura?=  writes:
> > No, no, no :) I wanted to do this, but from above reason I skipped it. I
> > swap VM pages, I do remap, in place where the shared buffer was I put
> > mmaped page, and in place where mmaped page was I put shared page (in
> > certain cases, which should be optimized by e. g. read for update, for
> > initial read of page in process I directly points to shared buffer), it
> > can be imagined as I affects TLB. This what I call "VM swap" is
> > remapping, so I don't change pointers, I change only where this pointers
> > points in physical memory, preserving same pointer in Virtual Memory.
> 
> ... Huh?  Are you saying that you ask the kernel to map each individual
> shared buffer separately?  I can't believe that's going to scale to
> realistic applications.
> 
>   regards, tom lane

No, I do 
mrempa(mmap_buff_A, MAP_FIXED, temp); 
mremap(shared_buff_Y, MAP_FIXED, mmap_buff_A), 
mrempa(tmp, MAP_FIXED, mmap_buff_A). 

This is this additional overhead - and may have some disadvantages. All 
regions SysV / Posix MMAP are mapped before. 

I couldn't believe too, but as I done some work about read, I was in dead 
corner: 
1. Create Read Before Buffer (connect with XLOG) that will store each page 
before modification (page should be flushed and synced to log)
2. Rewrite whole db to repoint pointers or similar stuff (I done few steps for 
this).
3. Or find something different. 

I couldn't believe too, it's way I still work on it. I saw it gains speed for 
few simple updates. I'm not quite sure why it gets it. I only may think it  
was from "pre update reads". But full checks will go after some good point of 
updates.

Regards,
Radek







-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Greg Smith]

Andrew Dunstan wrote:
What makes you think this isn't possible to run pgindent? There are no 
secret incantations.


The first hit newbies find looking for info about pgident is 
http://blog.hagander.net/archives/185-pgindent-vs-dash.html which sure 
looks like secret incantations to me.  The documentation 
src/tools/pgindent/README reads like a magic spell too:


   find . -name '*.[ch]' -type f -print | \
   egrep -v -f src/tools/pgindent/exclude_file_patterns | \
   xargs -n100 pgindent src/tools/pgindent/typedefs.list

And it doesn't actually work as written unless you've installed 
pgindent, entab/detab, and the specially patched NetBSD indent into the 
system PATH somewhere--unreasonable given that this may be executing on 
a source only tree that has never been installed..  The fact that the 
documention is only in the README and not with the rest of the code 
conventions isn't helping either.


The last time I tried to do this a few years ago I failed miserably and 
never came back.  I know way more about building software now though, 
and just got this to work for the first time.  Attached is a WIP wrapper 
script for running pgident that builds all the requirements into 
temporary directories, rather than expecting you to install anything 
system-wide or into a PostgreSQL destination directory.  Drop this into 
src/tools/pgindent, make it executable, and run it from that directory.  
Should do the right thing on any system that has "make" as an alias for 
"gmake" (TODO to be better about that in the file, with some other 
nagging things).


When I just ran it against master I got a bunch of modified files, but 
most of them look like things that have been touched recently so I think 
it did the right thing.  A test of my work here from someone who isn't 
running this for the first time would be helpful.  If this works well 
enough, I think it would make a good helper script to include in the 
distribution.  The loose ends to fix I can take care of easily enough 
once basic validation is finished.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


#!/bin/bash -ex

# Presume that we're in the pgindent directory at the start.
# TODO this should switch to the directory where this script is located at,
# in case someone calls src/tools/pgident/run-pgident

# Back to the base directory
pushd ../../..

# Sanity check we're in the right place
if [ ! -d src/tools/pgindent ] ; then
  echo run-pgindent can only be run from within the src/tools/pgindent 
directory, aborting
  popd
  exit 1
fi

echo pgindent setting up environment
wget ftp://ftp.postgresql.org/pub/dev/indent.netbsd.patched.tgz
mkdir -p indent
cd indent
zcat ../indent.netbsd.patched.tgz | tar xvf -
rm -f indent.netbsd.patched.tgz
make
INDENT_DIR=`pwd`
cd ..

pushd src/tools/entab directory
make
ln -s entab detab
ENTAB_DIR=`pwd`
popd

export PATH="$INDENT_DIR:$ENTAB_DIR:$PATH"

wget -O src/tools/pgindent/typedefs.list 
http://buildfarm.postgresql.org/cgi-bin/typedefs.pl

# This cleanup can only happen if there is already a makefile; assume
# that if there's isn't, this tree is clean enough
if [ -f GNUmakefile ] ; then
  # TODO this may need to be "gmake" on some systems instead
  make maintainer-clean
fi

echo pgindent starting run
find . -name '*.[ch]' -type f -print | \
  egrep -v -f src/tools/pgindent/exclude_file_patterns | \
  xargs -n100 src/tools/pgindent/pgindent src/tools/pgindent/typedefs.list

# Cleanup of utilities built temporarily here
unlink src/tools/entab/detab
rm -rf indent

popd
echo pgindent run complete

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Formatting Curmudgeons WAS: MMAP Buffers

[Magnus Hagander]

On Apr 17, 2011 8:17 AM, "Jeff Janes"  wrote:
>
> On 4/16/11, Andrew Dunstan  wrote:
> >
> >
> > What makes you think this isn't possible to run pgindent? There are no
> > secret incantations.
>
> A while ago I spent a few hours trying to run it and gave up.  I think
> it was something about needing some obscure BSD version of some tool
> which conflicted with just about everything else on the system.  I can
> try again and report back if anyone cares.

It does rely on BSD indent. For that very reason, we provide the source for
it, since most people have gnu indent. It's trivial to build, though, and
works just fine as a local build, and you can keep gnu indent as the main
one on your system - no conflicts.

It used to be a PITA due to the typedef list, but that has been fixed.
Perhaps we just need to document it a bit more...

/Magnus