Thread: GIT patch
Hi, I've started reading the GIT patch to see if I can help with the review. First thing I notice is that there are several things that seems left over; for example the comments in pg_proc for the new functions are incomplete. More subtle: in _bt_findinsertloc, the test for modifiedpage = _bt_groupify() may reset the bit set by the _bt_vacuum_one_page. Surely it should look like modifiedpage |= _bt_groupify() There's also a couple of spots that were not merged cleanly, but since they were inside comments, the compiler did not complain and so were not fixed. I'm also finding a certain lack of code commentary that makes the reviewing a bit harder. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Another thing that would be useful would be to separate the changes to add pgstat counters and view columns, since they are relatively minor and could be committed separately (or not at all for 8.3, even). -- Alvaro Herrera Developer, http://www.PostgreSQL.org/ <inflex> really, I see PHP as like a strange amalgamation of C, Perl, Shell <crab> inflex: you know that "amalgam" means "mixture with mercury", more or less, right? <crab> i.e., "deadly poison"
Oh, and the new function in bitmapset.c could use with some explanation of what it is. -- Alvaro Herrera http://www.amazon.com/gp/registry/5ZYLFMCVHXC "Es filósofo el que disfruta con los enigmas" (G. Coli)
Alvaro Herrera wrote: > I've started reading the GIT patch to see if I can help with the review. Thanks. > First thing I notice is that there are several things that seems left > over; for example the comments in pg_proc for the new functions are > incomplete. > > ... > I'm also finding a certain lack of code commentary that makes the > reviewing a bit harder. Sorry about that. As the patch stands, I tried to keep it as non-invasive as possible, with minimum changes to existing APIs. That's because in the winter we were discussing changes to the indexam API to support the bitmap index am, and also GIT. I wanted to just have a patch to do performance testing with, without getting into the API changes. I've been reluctant to spend time to clean up the code and comments, knowing that that's going to change a lot, depending on what kind of an API we settle on and what capabilities we're going to have in the executor. And also because there was no acceptance of even the general design, so it might just be rejected. Please read the discussions on the thread "bitmapscan changes": http://archives.postgresql.org/pgsql-patches/2007-03/msg00163.php There's basically three slightly alternative designs: 1. A grouped index tuple contains a bitmap of offsetnumbers, representing a bunch of heap tuples stored on the same heap page, that all have a key between the key stored on the index tuple and the next index tuple. We don't keep track of the ordering of the heap tuples represented by one group index tuple. When doing a normal, non-bitmap, index scan, they need to be sorted. This is what the patch currently implements. 2. Same as 1, but instead of storing the offsetnumbers in a bitmap, they're sorted in a list (variable-sized array, really), which keeps the ordering between the tuples intact. No sorting needed on index scans, and we can do binary searches using the list. But takes more space than a bitmap. 3. Same as 1, but mandate that all the heap tuples that are represented by the same grouped index tuple must be in index order on the heap page. If an out-of-order tuple is inserted, we need to split the grouped index tuple into two groups, to uphold that invariant. No sorting needed on index scans and we can do binary searches. But takes more space when the heap is not perfectly in order and makes the index to degrade into a normal b-tree more quickly when the table is updated. I'm leaning towards 2 or 3 myself at the moment, to keep it simple. In any case. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas wrote: > Alvaro Herrera wrote: > > I've started reading the GIT patch to see if I can help with the review. > As the patch stands, I tried to keep it as non-invasive as possible, > with minimum changes to existing APIs. That's because in the winter we > were discussing changes to the indexam API to support the bitmap index > am, and also GIT. I wanted to just have a patch to do performance > testing with, without getting into the API changes. Hmm, do say, doesn't it seem like the lack of feedback and the failed bitmap patch played against final development of this patch? At this point I feel like the patch still needs some work and reshuffling before it is in an acceptable state. The fact that there are some API changes for which the patch needs to be adjusted makes me feel like we should put this patch on hold for 8.4. So we would first get the API changes discussed and done and then adapt this patch to them. Of the three proposals you suggest, I think the first one > 1. A grouped index tuple contains a bitmap of offsetnumbers, > representing a bunch of heap tuples stored on the same heap page, that > all have a key between the key stored on the index tuple and the next > index tuple. We don't keep track of the ordering of the heap tuples > represented by one group index tuple. When doing a normal, non-bitmap, > index scan, they need to be sorted. This is what the patch currently > implements. makes the most sense -- the index is keep simple and fast, and doing the sorting during an indexscan seems a perfectly acceptable compromise, knowing that the amount of tuples possible returned for sort is limited by the heap blocksize. -- Alvaro Herrera http://www.amazon.com/gp/registry/5ZYLFMCVHXC "Everything that I think about is more fascinating than the crap in your head." (Dogbert's interpretationof blogger philosophy)
Alvaro Herrera wrote: > Hmm, do say, doesn't it seem like the lack of feedback and the failed > bitmap patch played against final development of this patch? Yes :(. That's a one reason why I tried to help with the review of that patch. > At this > point I feel like the patch still needs some work and reshuffling before > it is in an acceptable state. The fact that there are some API changes > for which the patch needs to be adjusted makes me feel like we should > put this patch on hold for 8.4. So we would first get the API changes > discussed and done and then adapt this patch to them. I hate to say it but I agree. Getting the API changes discussed and committed was my plan in February/March. Unfortunately it didn't happen back then. There's a few capabilities we need from the new API: 1. Support for candidate matches. Because a clustered index doesn't contain the key for every heap tuple, when you search for a value we don't know exactly which ones match. Instead, you get a bunch of candidate matches, which need to be rechecked after fetching the heap tuple. Oleg & Teodor pointed out that GiST could use the capability as well. I also proposed a while ago to change the hash index implementation so that it doesn't store the index key in the index, but just the hash of it. That again would need the support for candidate matches. And there's range-encoded bitmap indexes, if we implement them in a more distant future. 2. Support to sort the heap tuples represented by one index tuple, in normal index scans, if we go with alternative 1. Or support to do binary searches over them, if we go with alternative 2 or 3. As the patch stands, the sorting is done within b-tree, but that's quite ugly. > Of the three proposals you suggest, I think the first one > >> 1. A grouped index tuple contains a bitmap of offsetnumbers, >> representing a bunch of heap tuples stored on the same heap page, that >> all have a key between the key stored on the index tuple and the next >> index tuple. We don't keep track of the ordering of the heap tuples >> represented by one group index tuple. When doing a normal, non-bitmap, >> index scan, they need to be sorted. This is what the patch currently >> implements. > > makes the most sense -- the index is keep simple and fast, and doing the > sorting during an indexscan seems a perfectly acceptable compromise, > knowing that the amount of tuples possible returned for sort is limited > by the heap blocksize. The overhead is shown in the CPU test results, particularly in the select_range* tests, I put up on the git web site: http://community.enterprisedb.com/git/. The other alternatives might be simpler, though. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas wrote: > Alvaro Herrera wrote: > > Hmm, do say, doesn't it seem like the lack of feedback and the failed > > bitmap patch played against final development of this patch? > > Yes :(. That's a one reason why I tried to help with the review of that > patch. > > > At this > > point I feel like the patch still needs some work and reshuffling before > > it is in an acceptable state. The fact that there are some API changes > > for which the patch needs to be adjusted makes me feel like we should > > put this patch on hold for 8.4. So we would first get the API changes > > discussed and done and then adapt this patch to them. > > I hate to say it but I agree. Getting the API changes discussed and > committed was my plan in February/March. Unfortunately it didn't happen > back then. > > There's a few capabilities we need from the new API: > > 1. Support for candidate matches. Because a clustered index doesn't > contain the key for every heap tuple, when you search for a value we > don't know exactly which ones match. Instead, you get a bunch of > candidate matches, which need to be rechecked after fetching the heap > tuple. Oleg & Teodor pointed out that GiST could use the capability as > well. I also proposed a while ago to change the hash index > implementation so that it doesn't store the index key in the index, but > just the hash of it. That again would need the support for candidate > matches. And there's range-encoded bitmap indexes, if we implement them > in a more distant future. Well, then should we return to the review of your 'bitmapscan changes' patch ? I've posted a version which applies (or applied to the cvs head at the time of post) cleanly there: http://archives.postgresql.org/pgsql-patches/2007-06/msg00204.php > > 2. Support to sort the heap tuples represented by one index tuple, in > normal index scans, if we go with alternative 1. Or support to do binary > searches over them, if we go with alternative 2 or 3. As the patch > stands, the sorting is done within b-tree, but that's quite ugly. -- Alexey Klyukin http://www.commandprompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera wrote: > Heikki Linnakangas wrote: > > Alvaro Herrera wrote: > > > I've started reading the GIT patch to see if I can help with the review. > > > As the patch stands, I tried to keep it as non-invasive as possible, > > with minimum changes to existing APIs. That's because in the winter we > > were discussing changes to the indexam API to support the bitmap index > > am, and also GIT. I wanted to just have a patch to do performance > > testing with, without getting into the API changes. > > Hmm, do say, doesn't it seem like the lack of feedback and the failed > bitmap patch played against final development of this patch? At this > point I feel like the patch still needs some work and reshuffling before > it is in an acceptable state. The fact that there are some API changes > for which the patch needs to be adjusted makes me feel like we should > put this patch on hold for 8.4. So we would first get the API changes > discussed and done and then adapt this patch to them. As Heikki mentioned, this was discussed back in March/April with no movement. At this point we have at least a month until beta so please try to move it forward as much as possible. It isn't going to be any easier during 8.4. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Alexey Klyukin wrote: > Well, then should we return to the review of your 'bitmapscan changes' > patch ? I've posted a version which applies (or applied to the cvs head > at the time of post) cleanly there: > http://archives.postgresql.org/pgsql-patches/2007-06/msg00204.php Yes, that's probably a good place to start. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas <heikki@enterprisedb.com> writes:
> Alvaro Herrera wrote:
>> At this
>> point I feel like the patch still needs some work and reshuffling before
>> it is in an acceptable state. The fact that there are some API changes
>> for which the patch needs to be adjusted makes me feel like we should
>> put this patch on hold for 8.4. So we would first get the API changes
>> discussed and done and then adapt this patch to them.
> I hate to say it but I agree.
I concur with putting this whole area off till 8.4. We do not have any
consensus on what the API should be, which is exactly why the patch was
never finished. All the proposals are pretty ugly.
Another problem: frankly I'm pretty dissatisfied with the entire concept
of not storing all the index keys, especially in the proposed way which
would eliminate any outside control over whether keys are dropped or
not. Two problems I can see with it are:
1. The performance hit for functional indexes could be really steep,
since you'd need to recompute a potentially expensive function to
recheck matches.
2. This would forever cut off any development of indexscans that make
use of index key data beyond what btree itself knows how to do. An
example of the sort of thing I'm thinking about is applying a LIKE or
regex pattern match operator against the index key before visiting the
heap --- not just a derived >= or <= condition, but the actual pattern
match. We've discussed adding an index AM call that returns the key
values, which'd allow the executor to apply non-btree operators to them
before visiting the heap. But that idea is DOA if the planner can't
tell in advance whether the entries will be available.
So instead of pressing to try to get something into 8.3, I would rather
we stand back and think about it some more.
regards, tom lane
On Thu, 2007-08-02 at 16:12 -0400, Tom Lane wrote: > Heikki Linnakangas <heikki@enterprisedb.com> writes: > > Alvaro Herrera wrote: > >> At this > >> point I feel like the patch still needs some work and reshuffling before > >> it is in an acceptable state. The fact that there are some API changes > >> for which the patch needs to be adjusted makes me feel like we should > >> put this patch on hold for 8.4. So we would first get the API changes > >> discussed and done and then adapt this patch to them. > > > I hate to say it but I agree. > > I concur with putting this whole area off till 8.4. We do not have any > consensus on what the API should be, which is exactly why the patch was > never finished. All the proposals are pretty ugly. Given that about 40% of the remaining patch queue is GIT plus other related stuff, I would now agree and encourage others to as well. The benefits of bitmap and GIT indexes are high and many people will benefit if we make them available now. Poor long-term design of code is an important issue, but some people may not wish to wait. I would like to suggest that we open up the field a little more. Adding index types could be just as easy as adding datatypes. We have 2 index types under discussion here (GIT and bitmap) and another hacker working on a new form of R-Tree also. How hard will it be to add the infrastructure to allow new index types to be added to the server dynamically? Many aspects are already there, ISTM. We would gain potential access to the new index types, gain an important extension capability and it will still result in an earlier release of 8.3. This will then allow development of those index types to occur on pgfoundry. We can then fold back in the winners in the race to provide useful additional indexing capabilities. We may be surprised at the number of different alternatives people come forward with. Heck, I'd much rather have bitmap and/or GIT than hash indexes any day. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Simon Riggs wrote:
> On Thu, 2007-08-02 at 16:12 -0400, Tom Lane wrote:
> Heck, I'd much rather have bitmap and/or GIT than hash indexes any day.
But hash is all about the equality man!
- --
=== The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
Providing the most comprehensive PostgreSQL solutions since 1997 http://www.commandprompt.com/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFGuNLmATb/zqfZUUQRAorRAJ9D5KfQKhR4+5PgLOf0IGKf8JLU+wCgi5Z8
FlkAI8delKqik8hBtiezwc0=
=XmK9
-----END PGP SIGNATURE-----
"Simon Riggs" <simon@2ndquadrant.com> writes:
> How hard will it be to add the infrastructure to allow new index types
> to be added to the server dynamically?
INSERT INTO pg_am VALUES (...);
I don't really think we need more than that, at least not till non-core
index AMs are a whole lot thicker on the ground than they are today.
The real sticking point of course is the desired indexam API changes,
which can hardly be inserted dynamically. Your argument would work
better if there were not API issues to be resolved for both bitmap and
GIT ...
regards, tom lane
Tom Lane wrote: > Heikki Linnakangas <heikki@enterprisedb.com> writes: > > Alvaro Herrera wrote: > >> At this > >> point I feel like the patch still needs some work and reshuffling before > >> it is in an acceptable state. The fact that there are some API changes > >> for which the patch needs to be adjusted makes me feel like we should > >> put this patch on hold for 8.4. So we would first get the API changes > >> discussed and done and then adapt this patch to them. > > > I hate to say it but I agree. > > I concur with putting this whole area off till 8.4. We do not have any > consensus on what the API should be, which is exactly why the patch was > never finished. All the proposals are pretty ugly. > > Another problem: frankly I'm pretty dissatisfied with the entire concept > of not storing all the index keys, especially in the proposed way which > would eliminate any outside control over whether keys are dropped or > not. Two problems I can see with it are: > > 1. The performance hit for functional indexes could be really steep, > since you'd need to recompute a potentially expensive function to > recheck matches. > > 2. This would forever cut off any development of indexscans that make > use of index key data beyond what btree itself knows how to do. An > example of the sort of thing I'm thinking about is applying a LIKE or > regex pattern match operator against the index key before visiting the > heap --- not just a derived >= or <= condition, but the actual pattern > match. We've discussed adding an index AM call that returns the key > values, which'd allow the executor to apply non-btree operators to them > before visiting the heap. But that idea is DOA if the planner can't > tell in advance whether the entries will be available. > > So instead of pressing to try to get something into 8.3, I would rather > we stand back and think about it some more. I understand why you are saying hold for 8.4, but this issue came up in the middle of the 8.3 development cycle and didn't get much attention. I would like to know why it will get any more attention during 8.4. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Tom Lane wrote:
>> So instead of pressing to try to get something into 8.3, I would rather
>> we stand back and think about it some more.
> I understand why you are saying hold for 8.4, but this issue came up in
> the middle of the 8.3 development cycle and didn't get much attention.
> I would like to know why it will get any more attention during 8.4.
It's not "more attention" that it needs; it's "some good ideas". Which
we don't yet have, and we cannot produce on a schedule.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> So instead of pressing to try to get something into 8.3, I would rather > >> we stand back and think about it some more. > > > I understand why you are saying hold for 8.4, but this issue came up in > > the middle of the 8.3 development cycle and didn't get much attention. > > I would like to know why it will get any more attention during 8.4. > > It's not "more attention" that it needs; it's "some good ideas". Which > we don't yet have, and we cannot produce on a schedule. OK, I thought we were just waiting for a decision on proposed approaches, rather than new ideas, which certainly can take time. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Tom Lane wrote: > >> So instead of pressing to try to get something into 8.3, I would rather > >> we stand back and think about it some more. > > > I understand why you are saying hold for 8.4, but this issue came up in > > the middle of the 8.3 development cycle and didn't get much attention. > > I would like to know why it will get any more attention during 8.4. > > It's not "more attention" that it needs; it's "some good ideas". Which > we don't yet have, and we cannot produce on a schedule. This is a difficult email to write but it seems GIT isn't going to make it into 8.3. There seems to be too many open implementation questions to move forward. It seems the internal API changes need more thought. I somehow feel that if HOT wasn't being considered for 8.3 we might have gotten GIT, but with limited resources I think there was more focus on HOT, perhaps rightly so. These patches will be held for 8.4: o Grouped Index Tuples (GIT)o Bitmap scan changeso Stream bitmaps (API change for Group Index Tuples)o Maintaining clusterorder on insert I believe Heikki is in agreement on this. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Bruce Momjian wrote:
> Tom Lane wrote:
>> Bruce Momjian <bruce@momjian.us> writes:
> I somehow feel that if HOT wasn't being considered for 8.3 we might have
> gotten GIT, but with limited resources I think there was more focus on
> HOT, perhaps rightly so.
>
> These patches will be held for 8.4:
>
> o Grouped Index Tuples (GIT)
> o Bitmap scan changes
> o Stream bitmaps (API change for Group Index Tuples)
> o Maintaining cluster order on insert
>
> I believe Heikki is in agreement on this.
That is certainly a bummer.
Joshua D. Drake
- --
=== The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997 http://www.commandprompt.com/ UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFGxQU0ATb/zqfZUUQRArzSAJ4p6WKRiUEKOXsPduYViudNLvijDQCeMXJI
xvq7Ir1/bsQOpSlIqYwpYyc=
=kh8s
-----END PGP SIGNATURE-----
Joshua D. Drake wrote: > > These patches will be held for 8.4: > > > > o Grouped Index Tuples (GIT) > > o Bitmap scan changes > > o Stream bitmaps (API change for Group Index Tuples) > > o Maintaining cluster order on insert > > > > I believe Heikki is in agreement on this. > > That is certainly a bummer. I think text search has challenges similar to GIT, but the GIT issues were more how to change the internal API, while text search was a user-API issue which is easier to bang into shape. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Bruce Momjian wrote:
> Joshua D. Drake wrote:
>>> These patches will be held for 8.4:
>>>
>>> o Grouped Index Tuples (GIT)
>>> o Bitmap scan changes
>>> o Stream bitmaps (API change for Group Index Tuples)
>>> o Maintaining cluster order on insert
>>>
>>> I believe Heikki is in agreement on this.
>> That is certainly a bummer.
>
> I think text search has challenges similar to GIT, but the GIT issues
> were more how to change the internal API, while text search was a
> user-API issue which is easier to bang into shape.
Well let me just throw out there that I am in favor of this hold back
for 8.4. I don't like it but I am in favor of it.
Sincerely,
Joshua D. Drake
>
- --
=== The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 24x7/Emergency: +1.800.492.2240
PostgreSQL solutions since 1997 http://www.commandprompt.com/ UNIQUE NOT NULL
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate
PostgreSQL Replication: http://www.commandprompt.com/products/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFGxQftATb/zqfZUUQRAob2AJwKfNaUBgz6TmSI2/bCfYvbKwQmwgCfT7pg
6UXsRjC/4WPQM+zB93p4uPM=
=nXAo
-----END PGP SIGNATURE-----
Bruce Momjian wrote: > These patches will be held for 8.4: > > o Grouped Index Tuples (GIT) > o Bitmap scan changes > o Stream bitmaps (API change for Group Index Tuples) > o Maintaining cluster order on insert > > I believe Heikki is in agreement on this. Yes. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Tue, 2007-08-07 at 17:03 -0400, Tom Lane wrote: > "Simon Riggs" <simon@2ndquadrant.com> writes: > > How hard will it be to add the infrastructure to allow new index types > > to be added to the server dynamically? > > INSERT INTO pg_am VALUES (...); > > I don't really think we need more than that, at least not till non-core > index AMs are a whole lot thicker on the ground than they are today. We're able to dynamically add AMs in the way you suggest, but there is no way to alter the RMgrTable to either add a new RM or re-assign one of the unused RMs. So we can add a new AM, but it can't write WAL in a different way to existing RMs. We could either: 1. Remove the "Const" in front of RmgrTable in rmgr.c. That would allow re-assignment of the two existing unused RMs. 2. Create a new catalog table pg_rm and some brief machinery to populate the RmgrTable from pg_rm. That would allow dynamically adding RMs. Seems like we could do (1) for 8.3 easily enough. Thoughts, please? -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com
Simon Riggs <simon@2ndquadrant.com> writes:
> We're able to dynamically add AMs in the way you suggest, but there is
> no way to alter the RMgrTable to either add a new RM or re-assign one of
> the unused RMs.
Hmmm...
> 1. Remove the "Const" in front of RmgrTable in rmgr.c. That would allow
> re-assignment of the two existing unused RMs.
Useless, as there's no way for an add-on AM to cause the value to be
changed before recovery begins.
> 2. Create a new catalog table pg_rm and some brief machinery to populate
> the RmgrTable from pg_rm. That would allow dynamically adding RMs.
Also useless --- what if pg_rm is damaged or missing? Even if it's
there, how would recovery be able to tell which rows are valid? We
certainly don't want it trying to do pg_clog probes.
regards, tom lane
On Wed, 2007-09-19 at 10:37 -0400, Tom Lane wrote: > Simon Riggs <simon@2ndquadrant.com> writes: > > We're able to dynamically add AMs in the way you suggest, but there is > > no way to alter the RMgrTable to either add a new RM or re-assign one of > > the unused RMs. > > Hmmm... > > 1. Remove the "Const" in front of RmgrTable in rmgr.c. That would allow > > re-assignment of the two existing unused RMs. > > Useless, as there's no way for an add-on AM to cause the value to be > changed before recovery begins. OK, sounds like the only way is to have a dedicated plug-in. if (resource_manager_hook) set RmgrTable in plugin else normal static definition (but no longer Const) ...or variations of the above depending upon whether we want to allow redefining existing Rmgrs - not something I personally want. Plus something to say "xlog record found for unknown Rmgr". Plus changes to all places that refer to RM_MAX_ID and replace with a global value of RmgrTableEntries. We can then get rid of the two reserved Rmgr values... Will that do it? -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com