Subject: Re: AIX support

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 19 Jun 2024 18:15:14 +0300

On 19/06/2024 17:55, Srirama Kucherlapati wrote:
> +/* Commenting for XLC
> + * "IBM XL C/C++ for AIX, V12.1" miscompiles, for 32-bit, some inline
> + * expansions of ginCompareItemPointers() "long long" arithmetic.  To take
> + * advantage of inlining, build a 64-bit PostgreSQL.
> +#if defined(__ILP32__) && defined(__IBMC__)
> +#define PG_FORCE_DISABLE_INLINE
> +#endif
> + */

This seems irrelevant.

> + * Ordinarily, we'd code the branches here using GNU-style local symbols, that
> + * is "1f" referencing "1:" and so on.  But some people run gcc on AIX with
> + * IBM's assembler as backend, and IBM's assembler doesn't do local symbols.
> + * So hand-code the branch offsets; fortunately, all PPC instructions are
> + * exactly 4 bytes each, so it's not too hard to count.

Could you use GCC assembler to avoid this?

> @@ -662,6 +666,21 @@ tas(volatile slock_t *lock)
>  
>  #if !defined(HAS_TEST_AND_SET)	/* We didn't trigger above, let's try here */
>  
> +#if defined(_AIX)	/* AIX */
> +/*
> + * AIX (POWER)
> + */
> +#define HAS_TEST_AND_SET
> +
> +#include <sys/atomic_op.h>
> +
> +typedef int slock_t;
> +
> +#define TAS(lock)			_check_lock((slock_t *) (lock), 0, 1)
> +#define S_UNLOCK(lock)		_clear_lock((slock_t *) (lock), 0)
> +#endif	 /* _AIX */
> +
> +
>  /* These are in sunstudio_(sparc|x86).s */
>  
>  #if defined(__SUNPRO_C) && (defined(__i386) || defined(__x86_64__) || defined(__sparc__) || defined(__sparc))

What CPI/compiler/OS configuration is this for, exactly? Could we rely 
on GCC-provided __sync_lock_test_and_set() builtin function instead?

> +# Allow platforms with buggy compilers to force restrict to not be
> +# used by setting $FORCE_DISABLE_RESTRICT=yes in the relevant
> +# template.

Surely we don't need that anymore? Or is the compiler still buggy?

Do you still care about 32-bit binaries on AIX? If not, let's make that 
the default in configure or a check for it, and remove the instructions 
on building 32-bit binaries from the docs.

Please try hard to remove any changes from the diff that are not 
absolutely necessary.

- Heikki

---

From: Srirama Kucherlapati <[email-masked]>
Date: Wed, 19 Jun 2024 15:57:08 +0000

Thanks Hikki, for going through the changes.

> +#define PG_FORCE_DISABLE_INLINE
I can remove these unwanted comments.

I have to analyze the changes for the rest of your comment and will get back to you.

Warm regards,
Sriram.

From: Heikki Linnakangas <[email-masked]>
Date: Wednesday, 19 June 2024 at 8:45 PM
To: Srirama Kucherlapati <[email-masked]>, Laurenz Albe <[email-masked]>, Bruce Momjian <[email-masked]>, Heikki Linnakangas <[email-masked]>
Cc: Peter Eisentraut <[email-masked]>, Alvaro Herrera <[email-masked]>, [email-masked] <[email-masked]>, Noah Misch <[email-masked]>, Michael Paquier <[email-masked]>, Andres Freund <[email-masked]>, Tom Lane <[email-masked]>, Thomas Munro <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>
Subject: [EXTERNAL] Re: AIX support

This seems irrelevant.

- Heikki

---

From: Srirama Kucherlapati <[email-masked]>
Date: Fri, 21 Jun 2024 17:00:31 +0000

We are continuing to work on the changes…

  > on building 32-bit binaries from the docs.

As most of the products are moving towards 64bit, we will try to remove
this 32bit support.

Warm regards,
Sriram.

---

From: Srirama Kucherlapati <[email-masked]>
Date: Wed, 14 Aug 2024 03:31:21 +0000

Hi Heikki & Team,

I tried to look at the assembly code changes with our team, in the below file.

diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h

index 29ac6cdcd9..69582f4ae7 100644

--- a/src/include/storage/s_lock.h

+++ b/src/include/storage/s_lock.h

static __inline__ int

tas(volatile slock_t *lock)

@@ -424,17 +430,15 @@ tas(volatile slock_t *lock)

__asm__ __volatile__(

"        lwarx   %0,0,%3,1        \n"

"        cmpwi   %0,0                \n"

"        bne     $+16                \n"                /* branch to li %1,1 */

"        addi    %0,%0,1                \n"

"        stwcx.  %0,0,%3                \n"

"        beq     $+12                \n"                /* branch to lwsync */

"        li      %1,1                \n"

"        b       $+12                \n"                /* branch to end of asm sequence */

"        lwsync                                \n"

"        li      %1,0                \n"

:        "=&b"(_t), "=r"(_res), "+m"(*lock)

:        "r"(lock)

:        "memory", "cc");

For the changes in the above file,  this code is very specific to power architecture we need to use the IBM Power specific asm code only, rather than using the GNU assembler. Also, all these asm specific code is under the macro __ppc__, which should not impact any other platforms. I see there is a GCC specific implementation (under this macro #if defined(HAVE_GCC__SYNC_INT32_TAS)) in the same file as well.

The above changes are specific to AIX kernel and it operates on fixed kernel memory. This is more like a compare_and_swap functionality with sync capability. For all the assemble code I think it would be better to use the IBM Power specific asm code to gain additional performance.

I was trying to understand here wrt to both the assemble changes if you are looking for anything specific to the architecture.

Attached is the patch for the previous comments, kindly please let me know your comments.

Warm regards,
Sriram.

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 14 Aug 2024 10:46:38 +0300

On 14/08/2024 06:31, Srirama Kucherlapati wrote:
> For the changes in the above file,  this code is very specific to power 
> architecture we need to use the IBM Power specific asm code only, rather 
> than using the GNU assembler. Also, all these asm specific code is under 
> the macro __ppc__, which should not impact any other platforms. I see 
> there is a GCC specific implementation (under this macro #if 
> defined(HAVE_GCC__SYNC_INT32_TAS)) in the same file as well.

I'm sorry, I don't understand what you're saying here. Do you mean that 
we don't need to do anything here, and the code we have in s_lock.h in 
'master' now will work fine on AIX? Or do we need to (re-)do some 
changes to support AIX again? If we only support GCC, can we use the 
__sync_lock_test_and_set() builtin instead?

If any changes are required, please include them in the patch. That'll 
make it clear what exactly you're proposing.

> The above changes are specific to AIX kernel and it operates on fixed 
> kernel memory. This is more like a compare_and_swap functionality with 
> sync capability. For all the assemble code I think it would be better to 
> use the IBM Power specific asm code to gain additional performance.

You mean we don't need the above? Ok, good.

> I was trying to understand here wrt to both the assemble changes if you 
> are looking for anything specific to the architecture.

I don't know. You tell me what makes most sense on AIX / powerpc.

> Attached is the patch for the previous comments, kindly please let me 

Is this all that's needed to resurrect AIX support?

-- 
Heikki Linnakangas
Neon (https://neon.tech)

---

From: Srirama Kucherlapati <[email-masked]>
Date: Wed, 14 Aug 2024 15:22:18 +0000

Hi Heikki,
I have attached the merged patch with all the changes, the earlier patch was
just only the changes specific to older review comments.

    > __sync_lock_test_and_set() builtin instead?

Here we need these changes for ppc. These changes are not for enabling
the AIX support, but this is implementing “Enhanced PowerPC Architecture”.
This routine is more of compare_and_increment, which is different from
GCC __sync_lock_test_and_set(). Also I tried to write a sample function to
check the assemble generated by __sync_lock_test_and_set(), which turned out to
be different set of assemble code.

    > You mean we don't need the above? Ok, good.

I mean this part of the code is needed as this is specific to AIX kernel memory
operation which is different from __sync_lock_test_and_set().

I would like to mention that the changes made in src/include/storage/s_lock.h
are pretty much required and need to be operated in assemble specific to IBM
Power architecture.

Warm regards,
Sriram.

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 14 Aug 2024 23:09:06 +0300

On 14/08/2024 18:22, Srirama Kucherlapati wrote:
>      > I'm sorry, I don't understand what you're saying here. Do you 
> mean that
>      > we don't need to do anything here, and the code we have in 
> s_lock.h in
>      > 'master' now will work fine on AIX? Or do we need to (re-)do some
>      > changes to support AIX again? If we only support GCC, can we use the
>      > __sync_lock_test_and_set() builtin instead?
> 
> Here we need these changes for ppc. These changes are not for
> enabling the AIX support, but this is implementing “Enhanced PowerPC
> Architecture”. This routine is more of compare_and_increment, which
> is different from GCC __sync_lock_test_and_set(). Also I tried to
> write a sample function to check the assemble generated by
> __sync_lock_test_and_set(), which turned out to be different set of
> assemble code.

I still don't understand. We have Linux powerpc systems running happily 
in the buildfarm. They are happy with the current spinlock 
implementation. Why is this needed? What happens without it?

How is this different from __sync_lock_test_and_set()? Is the 
__sync_lock_test_and_set() on that platform broken, less efficient, or 
just different but equally good?

>      > > +#define TAS(lock)                      _check_lock((slock_t *) 
> (lock),
>      > > 0, 1)
>      > >
>      >>  +#define S_UNLOCK(lock)         _clear_lock((slock_t *) (lock), 0)
>      >>
>      > > The above changes are specific to AIX kernel and it operates on 
> fixed
>      > > kernel memory. This is more like a compare_and_swap 
> functionality with
>      > > sync capability. For all the assemble code I think it would be 
> better to
>      > > use the IBM Power specific asm code to gain additional performance.
> 
>      > You mean we don't need the above? Ok, good.
> 
> I mean this part of the code is needed as this is specific to AIX
> kernel memory operation which is different from
> __sync_lock_test_and_set().

How is it different from __sync_lock_test_and_set()? Why is it needed? 
What is AIX kernel memory operation?

> are pretty much required and need to be operated in assemble specific to IBM

Note that your patch both modifies the existing powerpc implementation, 
and introduces a new AIX-specific one. They cannot *both* be required, 
because only one of them will ever be compiled on a given platform. 
Which is it? Or are you trying to make this work on multiple different 
CPUs on AIX, so that different implementation gets chosen on different CPUs?

Is the mkldexport stuff still needed on modern AIX? Or was it specific 
to XLC and never needed on GCC? How do other products do that?

On a general note: it's your responsibility to explain all the changes 
in a way that others will understand and can verify. It is especially 
important for something critical and platform-specific like spinlocks, 
because others don't have easy access to the hardware to test these 
things independently. I also want to remind you that from the Postgres 
community point of view, you are introducing support for a new platform, 
AIX, not merely resurrecting the old stuff. Every change needs to be 
justified anew.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

---

From: Srirama Kucherlapati <[email-masked]>
Date: Wed, 11 Sep 2024 12:38:33 +0000

Hi Heikki and Team,

Thanks for your comments.

Here are some more details

    > I still don't understand. We have Linux powerpc systems
    > running happily in the buildfarm. They are happy with the
    > current spinlock implementation. Why is this needed?
    > What happens without it?
Not sure, by the time the below commits were made if there was a consideration
to use the gcc routines. My guess is that by using this PPC assembler code
would make the code independent of the compilers. Even the Linux ppc would use the
same asm. Since gcc is available on AIX, I have replaced the asm changes with
the gcc routine __sync_lock_test_and_set() to set the lock.

We have the gcc(package) build on the AIX platform and as part of the testsuite
there are no issues wrt this routine. We tried to test with the sample test
program extracted from gcc testsuite. Also we discussed the same with our
compiler teams internally and they see no issues using this routine.

    <attached sample test> gcc-12.4.0/libgomp/testsuite/libgomp.c/sections-1.c

--------

    > What is AIX kernel memory operation?

    More Info: _check_lock routine description
    https://www.ibm.com/docs/en/aix/7.1?topic=c-check-lock-subroutine
    The _check_lock subroutine performs an atomic (uninterruptible) sequence of
    operations. The compare_and_swap subroutine is similar, but does not issue
    synchronization instructions and therefore is inappropriate for updating lock
    words.

This change need not be replaced with gcc routine as, these changes will be
compiled for the non-gcc platforms only. This piece of code would never be
compiled, as we are using only gcc to build.

I tried to replace the AIX asm (under__ppc__ macro) with the gcc routine
__sync_lock_test_and_set(), and all the buildfarm tests passed. Attached patch
and the buildfarm output. Please let me know your feedback.

----

    > AIX, not merely resurrecting the old stuff. Every change needs to be

I do agree with you. To have a better understand on the previous changes,
I was going through the history of the file(s_lock.h) and see that multiple
changes that were made wrt the tas()routine specific to ppc/AIX. Below are
the commit levels.
I would kindly request, Tom Lane, to provide some insights on these changes.

    commit e3b06a871b63b90d4a08560ce184bb33324410b8
    commit 50938576d482cd36e52a60b5bb1b56026e63962a     << added tas() for AIX
    commit 7233aae50bea503369b0a4ef9a3b6a3864c96812
    commit ceb4f5ea9c2c6c2bd44d4799ff4a62c40a038894     << added tas() for PPC
    commit f9ba0a7fe56398e89fe349476d9e437c3197ea28
    commit eb5e4c58d137c9258eff5e41b09cb5fe4ed6d64c
    commit cd35d601b859d3a56632696b8d5293cbe547764b
    commit 109867748259d286dd01fce17d5f895ce59c68d5
    commit 5cfa8dd3007d7e953c6a03b0fa2215d97c581b0c
    commit 631beeac3598a73dee2c2afa38fa2e734148031b
    commit bc2a050d40976441cdb963ad829316c23e8df0aa
    commit c41a1215f04912108068b909569551f42059db29
    commit 50938576d482cd36e52a60b5bb1b56026e63962a

Please let me know if would like to try on the hardware, we have recently
setup a node in the OSU lab to try out.

Thanks,
Sriram.

---

From: Heikki Linnakangas <[email-masked]>
Date: Thu, 12 Sep 2024 00:57:33 +0300

On 11/09/2024 15:38, Srirama Kucherlapati wrote:
>> I still don't understand. We have Linux powerpc systems running
>> happily in the buildfarm. They are happy with the current spinlock
> Not sure, by the time the below commits were made if there was a 
> consideration to use the gcc routines.

The PPC asm code was originally written in 2002, and the first use of 
__sync_lock_test_and_set(), for ARM, appeared in 2012. The commit that 
made __sync_lock_test_and_set() be chosen automatically for platforms 
that don't specify anything else was added in 2022.

> I tried to replace the AIX asm (under__ppc__ macro) with the gcc
> routine __sync_lock_test_and_set(), and all the buildfarm tests
> passed. Attached patch and the buildfarm output. Please let me know
> your feedback.
Ok, if we don't need the assembler code at all, that's good. A patch to 
introduce AIX support should not change it for non-AIX powerpc systems 
though. That might be a good change, but would need to be justified 
separately, e.g.  by some performance testing, and should be a separate 
patch.

If you make no changes to s_lock.h at all, will it work? Why not?

> are pretty much required and need to be operated in assemble specific to IBM

Was that earlier statement incorrect? Is the manual wrong or outdated or 
not applicable to us?

Moving on..

Do you still need mkldexport.sh? Surely there's a better way to do that 
in year 2024. Some quick googling says there's a '-bexpall' option to 
'ld', which kind of sounds like what we want. Will that work? How do 
other programs do this?

-- 
Heikki Linnakangas
Neon (https://neon.tech)

---

From: Thomas Munro <[email-masked]>
Date: Thu, 12 Sep 2024 10:06:22 +1200

> If you make no changes to s_lock.h at all, will it work? Why not?

It's good to keep the work independent and I don't want to hold up
anything happening in this thread, but just for information: I have
been poking around at the idea of entirely removing the old spinlock
code and pointing spin.h's function-like-macros to the atomics code.
We couldn't do that before, because atomics were sometimes implemented
with spinlocks, but now that pg_atomic_flag is never implemented with
spinlocks we can flip that around, and then have only one place where
we know how to do this stuff.  What is needed for that to progress is,
I guess, to determine though assembler analysis or experimentation
across a bunch of targets that it works out at least as good...

https://www.postgresql.org/message-id/flat/CA%2BhUKGJ%2BoA%2B62iUZ-EQb5R2cAOW3Y942ZoOtzOD%3D1sQ05iNg6Q%40mail.gmail.com#23598cafac3dd08ca94fa9e2228a4764

---

From: Srirama Kucherlapati <[email-masked]>
Date: Fri, 13 Sep 2024 11:49:24 +0000

> _ sync_lock_test_and_set(), for ARM, appeared in 2012. The commit that
Thanks for the info.

------------------

    > If you make no changes to s_lock.h at all, will it work? Why not?
With the existing asm code I see there are some syntax errors, being hit.
But after reverting the old changes the issues resolved. Below are diffs.

     static __inline__ int
     tas(volatile slock_t *lock)
     {
    @@ -424,17 +413,15 @@ tas(volatile slock_t *lock)
            __asm__ __volatile__(
     "      lwarx   %0,0,%3,1       \n"
     "      cmpwi   %0,0            \n"
    -"      bne     1f                      \n"
    +"      bne     $+16            \n"             /* branch to li %1,1 */
     "      addi    %0,%0,1         \n"
     "      stwcx.  %0,0,%3         \n"
    -"      beq     2f                      \n"
    -"1: \n"
    +"      beq     $+12            \n"             /* branch to lwsync */
     "      li      %1,1            \n"
    -"      b       3f                      \n"
    -"2: \n"
    +"      b       $+12            \n"             /* branch to end of asm sequence */
     "      lwsync                          \n"
     "      li      %1,0            \n"
    -"3: \n"
    +
     :      "=&b"(_t), "=r"(_res), "+m"(*lock)
     :      "r"(lock)
     :      "memory", "cc");

Let me know if I need to run any perf tools to check the performance of
the __sync_lock_test_and_set change.

---------------

    > Was that earlier statement incorrect? Is the manual wrong or outdated or

Here this change is specific to AIX, but since we are compiling with gcc, this
is not applicable. But I will try with __sync* routines and check.

---------------
    > 'ld', which kind of sounds like what we want. Will that work? How do

Thanks for looking into this, I’m working on this, I will let you know.

Thanks,
Sriram.

---

From: Srirama Kucherlapati <[email-masked]>
Date: Tue, 17 Sep 2024 16:29:01 +0000

> 'ld', which kind of sounds like what we want. Will that work? How do

We have noticed couple of caveats with these flags -bexpall/-bexpfull in other
opensource tools on AIX.  This option would export too many symbols causing
problems because a shared library may re-export symbols from another library
causing confused dependencies, duplicate symbols.

We have similar discussion wrt to these flag in Cmake

    https://gitlab.kitware.com/cmake/cmake/-/issues/19163

Also, I tried some sample program to verify the same as below

    >> cat foo.c

        #include <stdio.h>

        #include <string.h>

        int func1()

        {

            char str1[] = "Hello ", str2[] = "world! ";

            strcat(str1,str2);

            puts(str1);

            return 0;

        }

    >> gcc -c foo.c -o foo.o

    >> gcc -shared -Wl,-bexpall -o foo.so foo.o

    >> dump -Tov foo.so

    foo.so:

                            ***Object Module Header***

    # Sections      Symbol Ptr      # Symbols       Opt Hdr Len     Flags

             4      0x00000d88            120                72     0x3002

    Flags=( EXEC DYNLOAD SHROBJ DEP_SYSTEM )

    Timestamp = "Sep 17 10:17:35 2024"

    Magic = 0x1df  (32-bit XCOFF)

                            ***Optional Header***

    Tsize       Dsize       Bsize       Tstart      Dstart

    0x00000548  0x0000010c  0x00000004  0x10000128  0x20000670

    SNloader    SNentry     SNtext      SNtoc       SNdata

    0x0004      0x0000      0x0001      0x0002      0x0002

    TXTalign    DATAalign   TOC         vstamp      entry

    0x0005      0x0004      0x20000750  0x0001      0xffffffff

    maxSTACK    maxDATA     SNbss       magic       modtype

    0x00000000  0x00000000  0x0003      0x010b        RE

                            ***Loader Section***

                            ***Loader Symbol Table Information***

    [Index]      Value      Scn     IMEX Sclass   Type           IMPid Name

    [0]     0x2000068c    .data              RW SECdef        [noIMid] __rtinit

    [1]     0x00000000    undef      IMP     DS EXTref libgcc_s.a(shr.o) __cxa_finalize

    [2]     0x00000000    undef      IMP     DS EXTref libgcc_s.a(shr.o) _GLOBAL__AIXI_shr_o

    [3]     0x00000000    undef      IMP     DS EXTref libgcc_s.a(shr.o) _GLOBAL__AIXD_shr_o

    [4]     0x00000000    undef      IMP     DS EXTref   libc.a(shr.o) __strtollmax

    [5]     0x00000000    undef      IMP     DS EXTref   libc.a(shr.o) puts

    [6]     0x200006f4    .data      EXP     DS   Ldef        [noIMid] __init_aix_libgcc_cxa_atexit

    [7]     0x20000724    .data      EXP     DS   Ldef        [noIMid] _GLOBAL__AIXI_foo_so

    [8]     0x20000730    .data      EXP     DS   Ldef        [noIMid] _GLOBAL__AIXD_foo_so

>>    [9]     0x2000073c    .data      EXP     DS SECdef        [noIMid] strcat

    [10]    0x20000744    .data      EXP     DS   Ldef        [noIMid] func1

The code makes use of strcat from libc but re-exports the symbol (because of -bexpall).

As of now due to the limitation with these flags (-bexpall / -bexpfull ? ), the

solution here is to continue to extract the symbols from the object files and

use that export file as part of building the shared library. (Continue to use

the mkldexport.sh script to generate the export symbols).

Thanks,
Sriram.

---

From: Srirama Kucherlapati <[email-masked]>
Date: Tue, 24 Sep 2024 11:25:35 +0000

Hi Heikki & team,

Could you please let me know your comments on the previous details?

Attached are the individual patches for AIX and gcc(__sync) routines.

Thanks,
Sriram.

---

From: Heikki Linnakangas <[email-masked]>
Date: Tue, 24 Sep 2024 16:09:39 +0300

Repeating what I said earlier:

> Ok, if we don't need the assembler code at all, that's good. A patch
> to introduce AIX support should not change it for non-AIX powerpc
> systems though. That might be a good change, but would need to be
> justified separately, e.g.  by some performance testing, and should
> be a separate patch
-- 
Heikki Linnakangas
Neon (https://neon.tech)

---

From: Srirama Kucherlapati <[email-masked]>
Date: Wed, 25 Sep 2024 06:04:45 +0000

Hi Heikki,

As requested earlier, I need some assistance from the Postgres side to identify any tool or testcase to calibrate the sync routine performance in Postgres.
I see the below tools for benchmarking.

  *   Pgbench https://www.postgresql.org/docs/current/pgbench.html
  *   Pg_test_fsync https://www.postgresql.org/docs/current/pgtestfsync.html
  *   pg_test_timing https://www.postgresql.org/docs/current/pgtesttiming.html

Please let me know, if these tools are fine or else ca you suggest us with any additional tools to run the benchmarking.

Thanks,
Sriram.

---

From: Srirama Kucherlapati <[email-masked]>
Date: Fri, 4 Oct 2024 15:04:57 +0000

Hi Heikki and team,

A few updates…

  > > separately, e.g.  by some performance testing, and should be a separate

We ran pgbench for both the patches on powerpc Linux and below are the test
results and both results are close.

PPCLE sync: is the patch with the spin lock using gcc __sync* routines.
PPCLE asm: is the patch with the assemble code.

>> pgbench -c 100 -p 5432 -d postgres -T 180 -r -P 10  -L 10 -j 20

    OS/type             : PPCLE sync        PPCLE asm
    ---------------------------------------------------------
    latency average     : 136.257           138.552
    ---------------------------------------------------------
    latency stddev      : 234.74            238.603
    ---------------------------------------------------------
    Initial
    connection time     : 101.791           88.411
    ---------------------------------------------------------
    TPS(without initial
     connection time)   : 733.633924        721.440648
    ---------------------------------------------------------
    No. of transactions
    actually processed  : 132080            129893
    ---------------------------------------------------------
    No of transactions
    above the 10.0 ms
    latency limit       : 124235/132080     122183/129893
                          (94.060%)         (94.064%)
    ---------------------------------------------------------

Please let us know your comments.
BTW we are working on the other review comments as well.

Warm regards,
Sriram.

---

From: Srirama Kucherlapati <[email-masked]>
Date: Mon, 25 Nov 2024 16:26:56 +0000

Hi Team,  here are few updates.

As of now we have removed all the old changes and made the changes that are
pretty much required by building from scratch. We had few issues with our
hardware as a result it took a while to build the code.

Below are the changes done as of now.

commit d2b4b4c2259e21ceaf05e393769b69728bfbee99 (HEAD -> master, origin/master, origin/HEAD)

>> git status
On branch master
Your branch is up to date with 'origin/master'.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        new file:   src/backend/port/aix/mkldexport.sh
        new file:   src/include/port/aix.h
        new file:   src/makefiles/Makefile.aix
        new file:   src/template/aix

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   Makefile
        modified:   configure
        modified:   doc/src/sgml/dfunc.sgml
        modified:   src/Makefile.shlib
        modified:   src/backend/Makefile
        modified:   src/backend/port/aix/mkldexport.sh
        modified:   src/backend/utils/error/elog.c
        modified:   src/include/storage/s_lock.h
        modified:   src/port/strerror.c
        modified:   src/test/regress/expected/jsonb_jsonpath.out

We are seeing this one issue as of now with ‘gmake check’…

  1 diff -U3 /home /postgres/src/test/regress/expected/jsonb_jsonpath.out /home/ /postgres/src/test/regress/results/jsonb_jsonpath.out
  2 --- /home /postgres/src/test/regress/expected/jsonb_jsonpath.out    2024-10-22 02:30:00.545814423 -0500
  3 +++ /home/ postgres/src/test/regress/results/jsonb_jsonpath.out 2024-11-12 03:31:52.251125056 -0600
  4 @@ -2687,7 +2687,7 @@
  5  select jsonb_path_query_tz('"12:34:56"', '$.time_tz().string()');
  6   jsonb_path_query_tz
  7  ---------------------
  8 - "12:34:56-07:00"
  9 + "12:34:56-08:00"
10  (1 row)
11
12  select jsonb_path_query('"12:34:56"', '$.time().string()');

After modifying the expected output for this testcase as below, the issue was
resolved and the test case passed. But we are trying to see the root cause of this.
"12:34:56-08:00"

We are working on the buildfarm setup to check the actual tests.

I shall keep you posted.

Warm regards,
Sriram.

---

From: Robert Haas <[email-masked]>
Date: Mon, 25 Nov 2024 16:02:12 -0500

> resolved and the test case passed. But we are trying to see the root cause of this.

This is not an AIX-specific issue. It was fixed in commit
af21152268317323480caa790c4a6347110f8085, committed October 30th.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Srirama Kucherlapati <[email-masked]>
Date: Thu, 28 Nov 2024 03:56:49 +0000

>> af21152268317323480caa790c4a6347110f8085, committed October 30th.

Thanks Robert, it worked after applying this change.

Warm regards,
Sriram.

---

From: Srirama Kucherlapati <[email-masked]>
Date: Wed, 4 Dec 2024 09:57:56 +0000

Hi Team, few more updates on the buildfarm, all the tests have passed and below
is the gist of the logs. We are working on the cleaning up the changes and will
post them for review.

        Tue Dec  3 04:08:29 2024: buildfarm run for CHANGEME:HEAD starting
        CHANGEME:HEAD          [04:09:49] running build ...
        ======== make log ===========
        CHANGEME:HEAD          [04:09:54] running basic regression tests ...
        ======== make check logs ===========
        CHANGEME:HEAD          [04:19:47] running make contrib ...
        ======== make contrib log ===========
        CHANGEME:HEAD          [04:19:48] running make testmodules ...
        ======== make testmodules log ===========
        CHANGEME:HEAD          [04:19:49] running install ...
        ======== make install log ===========
        CHANGEME:HEAD          [04:20:51] running make contrib install ...
        ======== make contrib install log ===========
        CHANGEME:HEAD          [04:21:20] running testmodules install ...
        ======== testmodules install log ===========
        CHANGEME:HEAD          [04:21:33] checking pg_upgrade
        ======== pg_upgrade check log ===========
        CHANGEME:HEAD          [04:21:33] checking test-decoding
        ======== test-decoding check log ===========
        ========================== output_iso/log/initdb.log ================
        ========================== output_iso/log/postmaster.log ================
        ========================== test_decoding/log/initdb.log ================
        ========================== test_decoding/log/postmaster.log ================
        CHANGEME:HEAD          [04:23:40] running make check miscellaneous modules ...
        =========== Module commit_ts check =============
        =========== Module injection_points check =============
        =========== Module test_oat_hooks check =============
        =========== Module test_slru check =============
        =========== Module unsafe_tests check =============
        =========== Module basic_archive check =============
        =========== Module pg_freespacemap check =============
        =========== Module pg_logicalinspect check =============
        =========== Module pg_stat_statements check =============
        =========== Module pg_walinspect check =============
        =========== Module test_decoding check =============
        CHANGEME:HEAD          [04:31:54] setting up db cluster (C)...
        ======== initdb log (C) ===========
        CHANGEME:HEAD          [04:32:13] starting db (C)...
        ======== start db (C) : 1 log ========
        =========== db log file ==========
        CHANGEME:HEAD          [04:32:14] running installcheck (C)...
        ======== make installcheck log ===========
        CHANGEME:HEAD          [04:44:14] restarting db (C)...
        ======== stop db (C): 1 log ==========
        =========== db log file ==========
        ======== start db (C) : 2 log ========
        =========== db log file ==========
        CHANGEME:HEAD          [04:44:16] running make isolation check ...
        ======== make isolation check logs ===========
        CHANGEME:HEAD          [04:48:22] restarting db (C)...
        ======== stop db (C): 2 log ==========
        =========== db log file ==========
        ======== start db (C) : 3 log ========
        =========== db log file ==========
        CHANGEME:HEAD          [04:49:24] running make PL installcheck (C)...
        ======== make pl installcheck log ===========
        CHANGEME:HEAD          [04:49:44] restarting db (C)...
        ======== stop db (C): 3 log ==========
        =========== db log file ==========
        ======== start db (C) : 4 log ========
        =========== db log file ==========
        CHANGEME:HEAD          [04:49:47] running make contrib installcheck (C)...
        ======== make contrib installcheck log ===========
        CHANGEME:HEAD          [04:55:01] restarting db (C)...
        ======== stop db (C): 4 log ==========
        =========== db log file ==========
        ======== start db (C) : 5 log ========
        =========== db log file ==========
        CHANGEME:HEAD          [04:55:09] running make test-modules installcheck (C)...
        ======== make testmodules installcheck log ===========
        CHANGEME:HEAD          [04:56:54] stopping db (C)...
        ======== stop db (C): 5 log ==========
        =========== db log file ==========
        CHANGEME:HEAD          [04:56:57] running make ecpg check ...
        ======== make ecpg check logs ===========
        CHANGEME:HEAD          [04:58:38] OK
        ======== log passed to send_result ===========

        ======== log passed to send_result ===========
        All stages succeeded

Generated logs files

aixpg-lpar01:~/build-farm-17/buildroot/HEAD/CHANGEME.fromsource-logs]
>> ls
build.log                        pl-install-check-C.log
check-pg_upgrade.log             startdb-C-1.log
check.log                        startdb-C-2.log
configure.log                    startdb-C-3.log
contrib-install-check-C.log      startdb-C-4.log
ecpg-check.log                   startdb-C-5.log
initdb-C.log                     stopdb-C-1.log
install-check-C.log              stopdb-C-2.log
install-contrib.log              stopdb-C-3.log
install-testmodules.log          stopdb-C-4.log
isolation-check.log              stopdb-C-5.log
make-contrib.log                 test-decoding-check.log
make-install.log                 testmodules-install-check-C.log
make-testmodules.log             web-txn.data
misc-check.log

Warm regards,
Sriram.