Re: [HACKERS] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
On Sat, Aug 23, 2014 at 3:32 AM, Alvaro Herrera wrote: > Great. Pushed. Thanks for the patch. There is a typo in what has been pushed. Patch attached. -- Michael diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d37534e..1bb46ea 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10722,7 +10722,7 @@ ATPrepChangeLoggedness(Relation rel, bool toLogged) /* * Scan conrelid if changing to permanent, else confrelid. This also - * determines whether an useful index exists. + * determines whether a useful index exists. */ ScanKeyInit(&skey[0], toLogged ? Anum_pg_constraint_conrelid : -- 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] [GSoC2014] Patch ALTER TABLE ... SET LOGGED
On Sat, Aug 23, 2014 at 8:53 AM, Michael Paquier
wrote:
>
> On Sat, Aug 23, 2014 at 3:32 AM, Alvaro Herrera
> wrote:
> > Great. Pushed. Thanks for the patch.
> There is a typo in what has been pushed. Patch attached.
>
Thanks... I fixed that in my last patch to change 'loggedness' to
'persistence'. Attached.
Regards,
--
FabrÃzio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d37534e..1d2fe1f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -152,7 +152,7 @@ typedef struct AlteredTableInfo
bool new_notnull; /* T if we added new NOT NULL constraints */
bool rewrite; /* T if a rewrite is forced */
Oid newTableSpace; /* new tablespace; 0 means no change */
- bool chgLoggedness; /* T if SET LOGGED/UNLOGGED is used */
+ bool chgPersistence; /* T if SET LOGGED/UNLOGGED is used */
char newrelpersistence; /* if above is true */
/* Objects to rebuild after completing ALTER TYPE operations */
List *changedConstraintOids; /* OIDs of constraints to rebuild */
@@ -388,8 +388,8 @@ static void change_owner_recurse_to_sequences(Oid relationOid,
static void ATExecClusterOn(Relation rel, const char *indexName,
LOCKMODE lockmode);
static void ATExecDropCluster(Relation rel, LOCKMODE lockmode);
-static bool ATPrepChangeLoggedness(Relation rel, bool toLogged);
-static void ATChangeIndexesLoggedness(Oid relid, char relpersistence);
+static bool ATPrepChangePersistence(Relation rel, bool toLogged);
+static void ATChangeIndexesPersistence(Oid relid, char relpersistence);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
char *tablespacename, LOCKMODE lockmode);
static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode);
@@ -3174,19 +3174,19 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_SetLogged: /* SET LOGGED */
ATSimplePermissions(rel, ATT_TABLE);
- tab->chgLoggedness = ATPrepChangeLoggedness(rel, true);
+ tab->chgPersistence = ATPrepChangePersistence(rel, true);
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
/* force rewrite if necessary */
- if (tab->chgLoggedness)
+ if (tab->chgPersistence)
tab->rewrite = true;
pass = AT_PASS_MISC;
break;
case AT_SetUnLogged: /* SET UNLOGGED */
ATSimplePermissions(rel, ATT_TABLE);
- tab->chgLoggedness = ATPrepChangeLoggedness(rel, false);
+ tab->chgPersistence = ATPrepChangePersistence(rel, false);
tab->newrelpersistence = RELPERSISTENCE_UNLOGGED;
/* force rewrite if necessary */
- if (tab->chgLoggedness)
+ if (tab->chgPersistence)
tab->rewrite = true;
pass = AT_PASS_MISC;
break;
@@ -3668,7 +3668,7 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
* Select persistence of transient table (same as original unless
* user requested a change)
*/
- persistence = tab->chgLoggedness ?
+ persistence = tab->chgPersistence ?
tab->newrelpersistence : OldHeap->rd_rel->relpersistence;
heap_close(OldHeap, NoLock);
@@ -3705,8 +3705,8 @@ ATRewriteTables(List **wqueue, LOCKMODE lockmode)
* because the rewrite step might read the indexes, and that would
* cause buffers for them to have the wrong setting.
*/
- if (tab->chgLoggedness)
-ATChangeIndexesLoggedness(tab->relid, tab->newrelpersistence);
+ if (tab->chgPersistence)
+ATChangeIndexesPersistence(tab->relid, tab->newrelpersistence);
/*
* Swap the physical files of the old and new heaps, then rebuild
@@ -4119,7 +4119,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
tab->relkind = rel->rd_rel->relkind;
tab->oldDesc = CreateTupleDescCopy(RelationGetDescr(rel));
tab->newrelpersistence = RELPERSISTENCE_PERMANENT;
- tab->chgLoggedness = false;
+ tab->chgPersistence = false;
*wqueue = lappend(*wqueue, tab);
@@ -10678,7 +10678,7 @@ ATExecGenericOptions(Relation rel, List *options)
* checks are skipped), otherwise true.
*/
static bool
-ATPrepChangeLoggedness(Relation rel, bool toLogged)
+ATPrepChangePersistence(Relation rel, bool toLogged)
{
Relation pg_constraint;
HeapTuple tuple;
@@ -10722,7 +10722,7 @@ ATPrepChangeLoggedness(Relation rel, bool toLogged)
/*
* Scan conrelid if changing to permanent, else confrelid. This also
- * determines whether an useful index exists.
+ * determines whether a useful index exists.
*/
ScanKeyInit(&skey[0],
toLogged ? Anum_pg_constraint_conrelid :
@@ -10792,7 +10792,7 @@ ATPrepChangeLoggedness(Relation rel, bool toLogged)
* given persistence.
*/
static void
-ATChangeIndexesLoggedness(Oid relid, char relpersistence)
+ATChangeIndexesPersistence(Oid relid, char relpersistence)
{
R
Re: [HACKERS] Parallel Sequence Scan doubts
On 08/21/2014 02:47 PM, Haribabu Kommi wrote: > Corrected subject. > > Hi Hackers, > > Implementation of "Parallel Sequence Scan" I think you mean "Parallel Sequential Scan". Scanning a sequence in parallel doesn't make much sense. > 1."Parallel Sequence Scan" can achieved by using the background > workers doing the job of actual sequence scan including the > qualification check also. Only if the qualifiers are stable/immutable I think. Not even necessarily stable functions - consider use of the fmgr/executor state contexts to carry information over between calls, and what parallel execution would do to that. > 3. In the executor Init phase, Try to copy the necessary data required > by the workers and start the workers. Copy how? Back-ends can only communicate with each other over shared memory, signals, and using sockets. Presumably you'd use a dynamic shared memory segment, but it's going to be "interesting" to copy that kind of state over. Some of the work Robert has proposed to add support for data structures that are native to dynamic shmem might help, I guess... > 4. In the executor run phase, just get the tuples which are sent by > the workers and process them further in the plan node execution. Again, how do you propose to copy these back to the main bgworker? That's been one of the things that's limited parallel scan when it's been looked at before. > 1. Data structures that are required to be copied from backend to > worker are currentTransactionState, Snapshot, GUC, ComboCID, Estate > and etc. That's a big "etc". Huge, in fact. Any function can reference any global variable. Even an immutable function might use globals for cache etc - and might, for example, set them up using an executor start hook. You cannot really make any assumptions about what functions access what memory. So it's not possible, as far as I can understand, to define a simple subset of state that must be copied. Nor is it necessarily correct to discard the copied state at the end of the run even if you can copy it. Code may well depend on that state being updated and preserved across calls. > I see some problems in copying "Estate" data structure into the shared > memory because it contains so many pointers. There is a need of some > infrastructure to copy these data structures into the shared memory. It's not just a matter of copying them into/via shmem. It's about their meaning. Does it even make sense to copy the executor state to another backend? If so, you can't copy it back, so what do you do at the end of the scans? > Any suggestions? Before you try to design anything more on this, study the *large* amount of discussion that has happened on this topic on this mailing list over the last years. This is not a simple or easy task, and it's not one you should approach without studying what's already been worked on, built, contemplated, etc. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- 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] proposal: rounding up time value less than its unit.
Tomonari Katsumata writes: > This patch rounds up the value when only it's less than required unit. > .. > Although my original complaint is fixed, I'm worried about this change will > make users confusing. Indeed. I have not understood why you are insisting on "round up" semantics. Wouldn't it make more sense for the behavior to be "round to nearest"? That would get rid of any worries about treating zero specially. > Is it better to raise a message(ex. INFO) when a value less than required > unit is set? No. Non-error messages are probably completely useless in this area: users will typically never see such messages for settings made in postgresql.conf, because it will not occur to them to look in the postmaster log. So the behavior needs to be self-explanatory without any messages; and that means it had better not be very complicated. 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] proposal: rounding up time value less than its unit.
Tom Lane-2 wrote > Tomonari Katsumata < > t.katsumata1122@ > > writes: >> This patch rounds up the value when only it's less than required unit. >> .. >> Although my original complaint is fixed, I'm worried about this change >> will >> make users confusing. > > Indeed. I have not understood why you are insisting on "round up" > semantics. Wouldn't it make more sense for the behavior to be "round to > nearest"? That would get rid of any worries about treating zero > specially. Wasn't the goal that all non-zero values result in the feature being enabled? With round nearest there will still be some values that are non-zero but that round to zero and thus disable the feature. Values failing in the range (0, 1) in the canonical unit must be treated specially otherwise we might as well just leave the current behavior as-is since floor is likely just as good a rule as round-nearest. For fractions greater than one round nearest is probably fine and indeed on average results in the least amount of potential adjustment magnitude. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816007.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- 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] proposal: rounding up time value less than its unit.
David G Johnston writes: > Tom Lane-2 wrote >> Indeed. I have not understood why you are insisting on "round up" >> semantics. Wouldn't it make more sense for the behavior to be "round to >> nearest"? That would get rid of any worries about treating zero >> specially. > Wasn't the goal that all non-zero values result in the feature being > enabled? With round nearest there will still be some values that are > non-zero but that round to zero and thus disable the feature. Ah. Okay, but then what's wrong with the original proposal of "use ceil() instead of floor()"? Basically I think the idea of treating fractions less than one differently from fractions greater than one is bogus; nobody will ever find that intuitive. Or we could adopt Peter's idea that zero shouldn't be special (instead using, say, -1 to turn things off). But I'm afraid that would break way too many peoples' configuration choices. 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] Removing dependency to wsock32.lib when compiling code on WIndows
On Fri, Aug 22, 2014 at 04:58:47PM +0900, Michael Paquier wrote: > Looking more into that, I am seeing that MinGW-32 is failing to find socket > at configure, contrary to MinGW-64. > > Here is what happens for MinGW-64 at configure: > configure:7638: checking for library containing socket > [...] > configure:7669: x86_64-w64-mingw32-gcc -o conftest.exe -Wall > -Wmissing-prototypes -Wpointer-arith \ > -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute > -Wformat-security -fno-stri\ > ct-aliasing -fwrapv -fexcess-precision=standard -g > -I./src/include/port/win32 -DEXEC_BACKEND -Wl\ > ,--allow-multiple-definition -Wl,--disable-auto-import conftest.c -lws2_32 > -lm >&5 > configure:7669: $? = 0 > configure:7686: result: -lws2_32 > > And for MinGW-32: > configure:7638: checking for library containing socket > [...] > configure:7669: gcc -o conftest.exe -Wall -Wmissing-prototypes > -Wpointer-arith -Wdeclaration-after\ > -statement -Wendif-labels -Wmissing-format-attribute -Wformat-security > -fno-strict-aliasing -fwrapv\ > -fexcess-precision=standard -g -I./src/include/port/win32 > -DEXEC_BACKEND -Wl,--allow-multiple-d\ > efinition -Wl,--disable-auto-import conftest.c -lws2_32 -lm >&5 > C:\Users\ioltas\AppData\Local\Temp\cciNV1Y8.o: In function `main': > c:\Users\ioltas\git\postgres/conftest.c:33: undefined reference to `socket' > collect2.exe: error: ld returned 1 exit status I see that ws2_32.def for 64-bit MinGW-w64 exports plain "socket", while 32-bit MinGW-w64 and 32-bit MinGW export "socket@12". In other words, 64-bit MinGW-w64 exports a "__cdecl socket" function, and the others export a "__stdcall socket" function. AC_SEARCH_LIBS doesn't work with __stdcall. > I am not sure which way is better, do we want HAVE_GETADDRINFO or > !HAVE_GETADDRINFO in all Windows builds? If we choose the former, we'll > need to understand why -lws2_32 is not added to LIBS for MinGW-32. If we > choose the latter, we would need to remove -lws2_32 from LIBS with MinGW-64 > for consistency with MinGW-32 I think. HAVE_GETADDRINFO is preferable whenever the OS functions getaddrinfo.h would replace are fully adequate. When PostgreSQL established its longstanding Windows behavior in this area, that was not so. A few generations of Windows have since gone out of support, so perhaps the situation has changed. -- 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] change alter user to be a true alias for alter role
Jov writes:
> I make the v2 of the patch,use Tom's advice.
> But I can't make ROLE and USER in the keyword list,it is hard to solve the
> conflict,or rewrite many gram rules.
This patch seems to be trying to make ROLE and USER interchangeable
*everywhere* in the grammar, which is moving the goalposts quite a long
way to little purpose. The idea seems rather misguided to me, since
certainly CREATE USER and CREATE ROLE will never be exact synonyms.
I think it's sufficient to solve the original complaint about them not
being interchangeable in ALTER.
The patch as given is broken anyway: it removes both those keywords
from the keyword lists, which effectively makes them fully reserved,
in fact worse than fully reserved.
What I had in mind was more like the attached, which I've not tested
but bison does compile it without complaint. I've not looked at the
documentation end of it, either.
regards, tom lane
*** src/backend/parser/gram.y~ Sat Aug 23 14:53:23 2014
--- src/backend/parser/gram.y Sat Aug 23 18:10:56 2014
*** static Node *makeRecursiveViewSelect(cha
*** 230,236
AlterFdwStmt AlterForeignServerStmt AlterGroupStmt
AlterObjectSchemaStmt AlterOwnerStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
! AlterCompositeTypeStmt AlterUserStmt AlterUserMappingStmt AlterUserSetStmt
AlterRoleStmt AlterRoleSetStmt
AlterDefaultPrivilegesStmt DefACLAction
AnalyzeStmt ClosePortalStmt ClusterStmt CommentStmt
--- 230,236
AlterFdwStmt AlterForeignServerStmt AlterGroupStmt
AlterObjectSchemaStmt AlterOwnerStmt AlterSeqStmt AlterSystemStmt AlterTableStmt
AlterTblSpcStmt AlterExtensionStmt AlterExtensionContentsStmt AlterForeignTableStmt
! AlterCompositeTypeStmt AlterUserMappingStmt
AlterRoleStmt AlterRoleSetStmt
AlterDefaultPrivilegesStmt DefACLAction
AnalyzeStmt ClosePortalStmt ClusterStmt CommentStmt
*** stmt :
*** 749,756
| AlterTSConfigurationStmt
| AlterTSDictionaryStmt
| AlterUserMappingStmt
- | AlterUserSetStmt
- | AlterUserStmt
| AnalyzeStmt
| CheckPointStmt
| ClosePortalStmt
--- 749,754
*** CreateUserStmt:
*** 1020,1029
*
* Alter a postgresql DBMS role
*
*/
AlterRoleStmt:
! ALTER ROLE RoleId opt_with AlterOptRoleList
{
AlterRoleStmt *n = makeNode(AlterRoleStmt);
n->role = $3;
--- 1018,1029
*
* Alter a postgresql DBMS role
*
+ * ALTER USER is accepted interchangeably with ALTER ROLE.
+ *
*/
AlterRoleStmt:
! ALTER role_or_user RoleId opt_with AlterOptRoleList
{
AlterRoleStmt *n = makeNode(AlterRoleStmt);
n->role = $3;
*** AlterRoleStmt:
*** 1033,1045
}
;
opt_in_database:
/* EMPTY */ { $$ = NULL; }
| IN_P DATABASE database_name { $$ = $3; }
;
AlterRoleSetStmt:
! ALTER ROLE RoleId opt_in_database SetResetClause
{
AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
n->role = $3;
--- 1033,1050
}
;
+ role_or_user:
+ ROLE
+ | USER
+ ;
+
opt_in_database:
/* EMPTY */ { $$ = NULL; }
| IN_P DATABASE database_name { $$ = $3; }
;
AlterRoleSetStmt:
! ALTER role_or_user RoleId opt_in_database SetResetClause
{
AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
n->role = $3;
*** AlterRoleSetStmt:
*** 1047,1053
n->setstmt = $5;
$$ = (Node *)n;
}
! | ALTER ROLE ALL opt_in_database SetResetClause
{
AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
n->role = NULL;
--- 1052,1058
n->setstmt = $5;
$$ = (Node *)n;
}
! | ALTER role_or_user ALL opt_in_database SetResetClause
{
AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
n->role = NULL;
*** AlterRoleSetStmt:
*** 1060,1095
/*
*
- * Alter a postgresql DBMS user
- *
- */
-
- AlterUserStmt:
- ALTER USER RoleId opt_with AlterOptRoleList
- {
- AlterRoleStmt *n = makeNode(AlterRoleStmt);
- n->role = $3;
- n->action = +1; /* add, if there are members */
- n->options = $5;
- $$ = (Node *)n;
- }
- ;
-
-
- AlterUserSetStmt:
- ALTER USER RoleId SetResetClause
- {
- AlterRoleSetStmt *n = makeNode(AlterRoleSetStmt);
- n->role = $3;
- n->database = NULL;
- n->setstmt = $4;
- $$ = (Node *)n;
- }
- ;
-
-
- /*
- *
*
Re: [HACKERS] proposal: rounding up time value less than its unit.
On Sat, Aug 23, 2014 at 9:38 PM, Tom Lane wrote: > > Ah. Okay, but then what's wrong with the original proposal of "use ceil() > instead of floor()"? Basically I think the idea of treating fractions > less than one differently from fractions greater than one is bogus; nobody > will ever find that intuitive. Or make it an error to specify a value that rounds to 0 but isn't 0. -- greg -- 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] proposal: rounding up time value less than its unit.
Tom Lane-2 wrote > David G Johnston < > david.g.johnston@ > > writes: >> Tom Lane-2 wrote >>> Indeed. I have not understood why you are insisting on "round up" >>> semantics. Wouldn't it make more sense for the behavior to be "round to >>> nearest"? That would get rid of any worries about treating zero >>> specially. > >> Wasn't the goal that all non-zero values result in the feature being >> enabled? With round nearest there will still be some values that are >> non-zero but that round to zero and thus disable the feature. > > Ah. Okay, but then what's wrong with the original proposal of "use ceil() > instead of floor()"? Basically I think the idea of treating fractions > less than one differently from fractions greater than one is bogus; nobody > will ever find that intuitive. > > Or we could adopt Peter's idea that zero shouldn't be special (instead > using, say, -1 to turn things off). But I'm afraid that would break way > too many peoples' configuration choices. Yes, using ceil() is the most acceptable, for me, solution that does not involve throwing an error. Are there any examples of where treating zero specially is a problem or is this concern theoretical? We've already decided to risk enabling disabled features by applying this patch since the likelihood of someone relying on the rounding to keep the feature disabled (or at its default value in some instances) is unlikely. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/proposal-rounding-up-time-value-less-than-its-unit-tp5811102p5816018.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- 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] Parallel Sequence Scan doubts
On Sun, Aug 24, 2014 at 1:11 AM, Craig Ringer wrote: > On 08/21/2014 02:47 PM, Haribabu Kommi wrote: >> Implementation of "Parallel Sequence Scan" > > I think you mean "Parallel Sequential Scan". Sorry for not being clear, Yes it is parallel sequential scan. >> 1."Parallel Sequence Scan" can achieved by using the background >> workers doing the job of actual sequence scan including the >> qualification check also. > > Only if the qualifiers are stable/immutable I think. > > Not even necessarily stable functions - consider use of the > fmgr/executor state contexts to carry information over between calls, > and what parallel execution would do to that. Thanks for your input. As of now we are targeting only immutable functions, that can be executed in parallel without any side effects. >> 3. In the executor Init phase, Try to copy the necessary data required >> by the workers and start the workers. > > Copy how? > > Back-ends can only communicate with each other over shared memory, > signals, and using sockets. Sorry for not being clear, copying those data structures into dynamic shared memory only. >From there the workers can access. >> 4. In the executor run phase, just get the tuples which are sent by >> the workers and process them further in the plan node execution. > > Again, how do you propose to copy these back to the main bgworker? With the help of message queues that are created in the dynamic shared memory, the workers can send the data to the queue. On other side the main backend receives the tuples from the queue. >> 1. Data structures that are required to be copied from backend to >> worker are currentTransactionState, Snapshot, GUC, ComboCID, Estate >> and etc. > > That's a big "etc". Huge, in fact. > > Any function can reference any global variable. Even an immutable > function might use globals for cache etc - and might, for example, set > them up using an executor start hook. You cannot really make any > assumptions about what functions access what memory. Yes you are correct. For that reason only I am thinking of Supporting of functions that only dependent on input variables and are not modifying any global data. >> I see some problems in copying "Estate" data structure into the shared >> memory because it contains so many pointers. There is a need of some >> infrastructure to copy these data structures into the shared memory. > > It's not just a matter of copying them into/via shmem. > > It's about their meaning. Does it even make sense to copy the executor > state to another backend? If so, you can't copy it back, so what do you > do at the end of the scans? If we handle the locking of relation in the backend and avoid doing the parallel sequential scan if any sub query is involved, then there is no need of full estate in the worker. In those cases by sharing less information, I think we can execute the plan in the worker. >> Any suggestions? > > Before you try to design anything more on this, study the *large* amount > of discussion that has happened on this topic on this mailing list over > the last years. > > This is not a simple or easy task, and it's not one you should approach > without studying what's already been worked on, built, contemplated, etc. Thanks for your information. Regards, Hari Babu Fujitsu Australia -- 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] Parallel Sequence Scan doubts
On 08/24/2014 09:40 AM, Haribabu Kommi wrote:
> Any suggestions?
Another point I didn't raise first time around, but that's IMO quite
significant, is that you haven't addressed why this approach to fully
parallel seqscans is useful and solves real problems in effective ways.
It might seem obvious - "of course they're useful". But I see two things
they'd address:
- CPU-limited sequential scans, where expensive predicates are filtering
the scan; and
- I/O limited sequential scans, where the predicates already execute
fast enough on one CPU, so most time is spent waiting for more disk I/O.
The problem I see with your design is that it's not going to be useful
for a large class of CPU-limited scans where the predicate isn't
composed entirely of immutable functions an operators. Especially since
immutable-only predicates are the best candidates for expression indexes
anyway.
While it'd likely be useful for I/O limited scans, it's going to
increase contention on shared_buffers locking and page management. More
importantly, is it the most efficient way to solve the problem with I/O
limited scans?
I would seriously suggest looking at first adding support for
asynchronous I/O across ranges of extents during a sequential scan. You
might not need multiple worker backends at all.
I'm sure using async I/O to implement effective_io_concurrency for
seqscans has been been discussed and explored before, so again I think
some time in the list archives might make sense.
I don't know if it makes sense to do something as complex and parallel
multi-process seqscans without having a path forward for supporting
non-immutable functions - probably with fmgr API enhancements,
additional function labels ("PARALLEL"), etc, depending on what you find
is needed.
Do you have specific workloads where you see this as useful, and where
doing async I/O and readahead within a single back-end wouldn't solve
the same problem?
>>> 3. In the executor Init phase, Try to copy the necessary data required
>>> by the workers and start the workers.
>>
>> Copy how?
>>
>> Back-ends can only communicate with each other over shared memory,
>> signals, and using sockets.
>
> Sorry for not being clear, copying those data structures into dynamic
> shared memory only.
> From there the workers can access.
That'll probably work with read-only data, but it's not viable for
read/write data unless you use a big lock to protect it, in which case
you lose the parallelism you want to achieve.
You'd have to classify what may be modified during scan execution
carefully and determine if you need to feed any of the resulting
modifications back to the original backend - and how to merge
modifications by multiple workers, if it's even possible.
That's going to involve a detailed structure-by-structure analysis and
seems likely to be error prone and buggy.
I think you should probably talk to Robert Haas about what he's been
doing over the last couple of years on parallel query.
>>> 4. In the executor run phase, just get the tuples which are sent by
>>> the workers and process them further in the plan node execution.
>>
>> Again, how do you propose to copy these back to the main bgworker?
>
> With the help of message queues that are created in the dynamic shared memory,
> the workers can send the data to the queue. On other side the main
> backend receives the tuples from the queue.
OK, so you plan to implement shmem queues.
That'd be a useful starting point, as it'd be something that would be
useful in its own right.
You'd have to be able to handle individual values that're than the ring
buffer or whatever you're using for transfers, in case you're dealing
with already-detoasted tuples or in-memory tuples.
Again, chatting with Robert and others who've worked on dynamic shmem,
parallel query, etc would be wise here.
> Yes you are correct. For that reason only I am thinking of Supporting
> of functions
> that only dependent on input variables and are not modifying any global data.
You'll want to be careful with that. Nothing stops an immutable function
referencing a cache in a C global that it initializes one and then
treats as read only, for example.
I suspect you'll need a per-function whitelist. I'd love to be wrong.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
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] [BUGS] BUG #9652: inet types don't support min/max
On Mon, Aug 18, 2014 at 8:12 PM, Asif Naeem wrote: > Thank you for sharing updated patch. With latest 9.5 source code, patch > build is failing with following error message i.e. > >> /Applications/Xcode.app/Contents/Developer/usr/bin/make -C catalog >> schemapg.h >> cd ../../../src/include/catalog && '/opt/local/bin/perl' ./duplicate_oids >> 3255 >> make[3]: *** [postgres.bki] Error 1 >> make[2]: *** [submake-schemapg] Error 2 >> make[1]: *** [all-backend-recurse] Error 2 >> make: *** [all-src-recurse] Error 2 > > > New function is being added by following commit i.e. > >> commit b34e37bfefbed1bf9396dde18f308d8b96fd176c >> Author: Robert Haas >> Date: Thu Aug 14 12:09:52 2014 -0400 >> Add sortsupport routines for text. >> This provides a small but worthwhile speedup when sorting text, at >> least >> in cases to which the sortsupport machinery applies. >> Robert Haas and Peter Geoghegan > > > It seems that you need to use another oid. Other than this patch looks good > to me, please share updated patch and feel free to assign it to committer. > Thanks. Thanks for your review. Please find the rebased patch to latest HEAD. Regards, Hari Babu Fujitsu Australia inet_agg_v8.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Missing comment block at the top of streamutil.h and receivelog.h
Hi all, As mentioned in $subject, the header files in src/bin/pg_basebackup do not have a comment block at the top and do not have any copyright text. Any reason for that? Shouldn't we have something for consistency with the other files like in the patch attached? Regards, -- Michael diff --git a/src/bin/pg_basebackup/receivelog.h b/src/bin/pg_basebackup/receivelog.h index 72f8245..cd02d99 100644 --- a/src/bin/pg_basebackup/receivelog.h +++ b/src/bin/pg_basebackup/receivelog.h @@ -1,3 +1,16 @@ +/*- + * + * receivelog.h + * + * Author: Magnus Hagander + * + * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/bin/pg_basebackup/receivelog.h + *- + */ + #include "libpq-fe.h" #include "access/xlogdefs.h" diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h index c36a37b..a37d9cd 100644 --- a/src/bin/pg_basebackup/streamutil.h +++ b/src/bin/pg_basebackup/streamutil.h @@ -1,3 +1,16 @@ +/*- + * + * streamutil.h + * + * Author: Magnus Hagander + * + * Portions Copyright (c) 1996-2014, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/bin/pg_basebackup/streamutil.h + *- + */ + #include "libpq-fe.h" extern const char *progname; -- 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] TODO : Allow parallel cores to be used by vacuumdb [ WIP ]
On Tue, Aug 19, 2014 at 4:27 PM, Amit Kapila
wrote:
>
> Few more comments:
>
Some more comments:
1. I could see one shortcoming in the way the patch has currently
parallelize the
work for --analyze-in-stages. Basically patch is performing the work for
each stage
for multiple tables in concurrent connections that seems okay for the
cases when
number of parallel connections is less than equal to number of tables,
but for
the case when user has asked for more number of connections than number
of tables,
then I think this strategy will not be able to use the extra
connections.
2. Similarly for the case of multiple databases, currently it will not be
able
to use connections more than number of tables in each database because
the
parallelizing strategy is to just use the conncurrent connections for
tables inside single database.
I am not completely sure whether current strategy is good enough or
we should try to address the above problems. What do you think?
3.
+ do
+ {
+ i = select_loop(maxFd, &slotset);
+ Assert(i != 0);
Could you explain the reason of using this loop, I think you
want to wait for data on socket descriptor, but why for maxFd?
Also it is better if you explain this logic in comments.
4.
+ for (i = 0; i < max_slot; i++)
+ {
+ if (!FD_ISSET(pSlot[i].sock, &slotset))
+ continue;
+
+ PQconsumeInput(pSlot[i].connection);
+ if (PQisBusy(pSlot[i].connection))
+ continue;
I think it is better to call PQconsumeInput() only if you find
connection is busy.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
