Tignor, Tom ttignor at akamai.com
Wed Aug 8 09:59:44 PDT 2018
	Just checked and it looks good. Thanks!

	Tom    (


On 8/8/18, 11:43 AM, "Steve Singer" <steve at ssinger.info> wrote:

    On Tue, 7 Aug 2018, Tignor, Tom wrote:
    
    >
    > 	Understood. It would be fine to use a different env var.
    >
    > 	Tom     (
    
    
    I've updated the branch in github to use a different environment variable.
    Let me know if that looks okay
    
    >
    > On 8/7/18, 5:21 PM, "Steve Singer" <steve at ssinger.info> wrote:
    >
    >    On Tue, 7 Aug 2018, Tignor, Tom wrote:
    >
    >    >
    >    > 	Hi Steve,
    >    > 	Followed the github link and found a space to add comments there, so I did. Feel free to discuss here or there as you prefer. Thanks.
    >    >
    >    > 	Tom    (
    >
    >    My concern with PGHOME is that this environment variable might be set in
    >    existing installations to a  different directory that where slonik is
    >    picking up the .sql files today.
    >
    >    How do you feel about using a new environmental variable, SLONY_SHAREDIR or
    >    something like that?
    >
    >
    >
    >
    >    >
    >    >
    >    > On 8/1/18, 4:08 PM, "Steve Singer" <steve at ssinger.info> wrote:
    >    >
    >    >    On Wed, 6 Jun 2018, Steve Singer wrote:
    >    >
    >    >    > On Fri, 25 May 2018, Tignor, Tom wrote:
    >    >    >
    >    >    > Maybe this switch shouldn't be based on a compile-time conditional?
    >    >    >
    >    >    > Should this switch really be a command line argument instead of an
    >    >    > environment variable such as
    >    >    >
    >    >    > slonik --pg-share-dir=/usr/local/pg95/share
    >    >    >
    >    >    > And if the argument is set we use that path ?
    >    >
    >    >    Hearing no objections or better ideas I've done what I've described above.
    >    >    (However the option is -s not a long form).
    >    >
    >    >    See
    >    >    https://github.com/ssinger/slony1-engine/commit/60266f0dec3f0e88c2741ec37320bdd66b74314d
    >    >
    >    >    My plan is to merge this branch into REL_2_2_STABLE and master.
    >    >
    >    >    I hope to release 2.2.7 early next week, or the following week with this and
    >    >    the other patches included.
    >    >
    >    >
    >    >
    >    >
    >    >    >
    >    >    >
    >    >    >>
    >    >    >>
    >    >    >> 	Tom    (
    >    >    >>
    >    >    >>
    >    >    >> On 5/24/18, 7:27 AM, "Tignor, Tom" <ttignor at akamai.com> wrote:
    >    >    >>
    >    >    >>
    >    >    >>    	Great! Thank you. I can write up something for the options. I'll find
    >    >    >> config options in the existing doc and come up with something similar. Let
    >    >    >> me know if there's anything else I should do.
    >    >    >>    	What version do you expect these changes to first appear in?
    >    >    >>
    >    >    >>    	Tom    (
    >    >    >>
    >    >    >>
    >    >    >>    On 5/23/18, 10:15 PM, "Steve Singer" <steve at ssinger.info> wrote:
    >    >    >>
    >    >    >>        On Thu, 26 Apr 2018, Tignor, Tom wrote:
    >    >    >>
    >    >    >>        >
    >    >    >>        > 	Hello Steve and Slony-I,
    >    >    >>        > 	After much discussion, Akamai has decided to assign copyright
    >    >    >> for the changes discussed here to me. I'm now contributing the changes as
    >    >    >> myself. Please find the work for the previous changes consolidated and
    >    >    >> attached as one "options" patch. The other issues discussed were also
    >    >    >> addressed. Please take another look when time allows. What would be the
    >    >    >> next steps here?
    >    >    >>        > 	Thanks,
    >    >    >>        >
    >    >    >>        > 	Tom    (
    >    >    >>        >
    >    >    >>
    >    >    >>        I've looked over the patch.
    >    >    >>        It looks fine (I've changed some indentation)
    >    >    >>
    >    >    >>        I am okay merging this
    >    >    >> https://github.com/ssinger/slony1-engine/tree/akamai
    >    >    >>        into master.
    >    >    >>
    >    >    >>        However we do need to also update the admin guide to describe the
    >    >    >> new
    >    >    >>        options.
    >    >    >>
    >    >    >>
    >    >    >>
    >    >    >>        >
    >    >    >>        > On 2/3/18, 4:15 PM, "Steve Singer" <steve at ssinger.info> wrote:
    >    >    >>        >
    >    >    >>        >    On Tue, 23 Jan 2018, Tignor, Tom wrote:
    >    >    >>        >
    >    >    >>        >    >    compatibility_v2.2.patch
    >    >    >>        >    >
    >    >    >>        >    >    My understanding of this patch is so that you slon for any
    >    >    >> 2.2.x version to
    >    >    >>        >    >    be able to connect and replicate against a backend
    >    >    >> database with any other
    >    >    >>        >    >    2.2.y version (where x and y can be, but are not required
    >    >    >> to be equal).
    >    >    >>        >    >
    >    >    >>        >    > ttignor – Yes, that’s the intention and I believe the exact
    >    >    >> effect.
    >    >    >>        >    > Specifically, we’re upgrading our slony1-dependent services
    >    >    >> from
    >    >    >>        >    > slony1-2.2.4 to slony1-2.2.6. The upgrade process we’ve
    >    >    >> verified requires
    >    >    >>        >    > a mixed mode environment for a limited period of time (a few
    >    >    >> days). I
    >    >    >>        >    > chose “2.2.x” arbitrarily. We could achieve what we need with
    >    >    >> more
    >    >    >>        >    > specific criteria, or a config option, or a combination of
    >    >    >> the two.
    >    >    >>        >    > --------
    >    >    >>        >
    >    >    >>        >    How do you feel about instead having an option to disable the
    >    >    >> slony version
    >    >    >>        >    check.
    >    >    >>        >
    >    >    >>        >    My concern with saying all 2.2.x versions are compatible or
    >    >    >> even that
    >    >    >>        >    certain minor/patch versions are compatible is the cross
    >    >    >> version testing
    >    >    >>        >    load it puts on slony maintainers and how it might restrict
    >    >    >> things in the
    >    >    >>        >    future.
    >    >    >>        >
    >    >    >>        >    If instead we just provide a option to disable the version
    >    >    >> check users can
    >    >    >>        >    use this option as they see fit.
    >    >    >>        >
    >    >    >>        >
    >    >    >>        >
    >    >    >>        >    >
    >    >    >>        >    > A few things on the specific patch
    >    >    >>        >    >
    >    >    >>        >    >    +++ slony1-2.2.6/src/slon/dbutils.c     2017-12-22
    >    >    >> 08:08:33.027322631 -0500
    >    >    >>        >    >    @@ -5,6 +5,7 @@
    >    >    >>        >    >       *
    >    >    >>        >    >       *     Copyright (c) 2003-2009, PostgreSQL Global
    >    >    >> Development Group
    >    >    >>        >    >       *     Author: Jan Wieck, Afilias USA INC.
    >    >    >>        >    >    + *      Copyright (C) 2017 - Akamai Technologies, Inc
    >    >    >>        >    >       *
    >    >    >>        >    >       *
    >    >    >>        >    >       *
    >    >    >> ----------------------------------------------------------------------
    >    >    >>        >    >    @@ -418,7 +419,8 @@
    >    >    >>        >    >
    >    >    >>        >    >    We don't normally list contributors in the copyright
    >    >    >> section. The copyright
    >    >    >>        >    >    to PostgreSQL global development group is intended to
    >    >    >> cover all
    >    >    >>        >    >    contributors.
    >    >    >>        >    >
    >    >    >>        >    > ttignor – I’ve gone a few rounds on this with our opensource
    >    >    >> group. As I understand the issues, there is a choice to either maintain or
    >    >    >> assign away the Akamai copyright for the code I wrote.  Per our previous
    >    >    >> discussion, we’re maintaining the Akamai copyright. This is accomplished by
    >    >    >> header comments and/or some meta-data in the patch. If the header comment
    >    >    >> is a problem, then the patch meta-data becomes essential. If slony1-hackers
    >    >    >> can agree on exactly what copyright changes are needed, I can take them
    >    >    >> back to Akamai opensource.
    >    >    >>        >    > --------
    >    >    >>        >    >
    >    >    >>        >    > So for the pg_home patch.
    >    >    >>        >    > …
    >    >    >>        >    >  #else
    >    >    >>        >    > +       char *pgHome = getenv("PG_HOME");
    >    >    >>        >    > +       if (pgHome) {
    >    >    >>        >    > +         strncpy(share_path, pgHome, MAXPGPATH-1);
    >    >    >>        >    > +         share_path[MAXPGPATH-1] = '\0';
    >    >    >>        >    > +         strncat(share_path, "/share",
    >    >    >> MAXPGPATH-1-strlen(pgHome));
    >    >    >>        >    > +       } else {
    >    >    >>        >    >         strcpy(share_path, PGSHARE);
    >    >    >>        >    > +       }
    >    >    >>        >    >
    >    >    >>        >    >
    >    >    >>        >    > The above code only gets compiled in if PGPORT is not
    >    >    >> defined/present at
    >    >    >>        >    > build time.  Is that your intention,  or do you want PG_HOME
    >    >    >> to take
    >    >    >>        >    > precendence even if pgport is present?
    >    >    >>        >    >
    >    >    >>        >    > snprintf(share_path,"%s/share",pgHome,MAXPGPATH-1);
    >    >    >>        >    >
    >    >    >>        >    > Would the above code be clearer? (I haven't tested/tried to
    >    >    >> compile above)
    >    >    >>        >    > but trying to do this in one line?
    >    >    >>        >    >
    >    >    >>        >    > ttignor – Reviewing the changes, the HAVE_PGPORT clause is
    >    >    >> marked with a comment “We need to find a share directory like PostgreSQL.”.
    >    >    >> That seemed like something I shouldn’t override with a customization. Re:
    >    >    >> snprintf, that seems like a good improvement. I see snprintf can also
    >    >    >> truncate output, so I’ll need to account for that as well with a rewrite.
    >    >    >>        >    > --------
    >    >    >>        >    >
    >    >    >>        >    > 	More thoughts from Steve or others? Keep them coming.
    >    >    >>        >    >
    >    >    >>        >    > 	Tom    (
    >    >    >>        >    >
    >    >    >>        >    >
    >    >    >>        >    >
    >    >    >>        >    >
    >    >    >>        >    >
    >    >    >>        >    >
    >    >    >>        >    >
    >    >    >>        >
    >    >    >>        >
    >    >    >>        >
    >    >    >>
    >    >    >>
    >    >    >>
    >    >    >>
    >    >    >>
    >    >    >
    >    >
    >    >
    >    >
    >
    >
    >
    



More information about the Slony1-hackers mailing list