Repository navigation
Conversation
PR #2 adapted pljava-so for PG19 but left one change ungated, so the shared whpg branch no longer compiled against WarehousePG 7 (PG12): TupleTable.c: SPITupleTable.numvals is a PostgreSQL 13 addition ("Further adjust SPITupleTable to provide a public row-count field"), which also dropped the free member. PG12 still has the alloced/free pair, so the row count is now selected on PG_VERSION_NUM >= 130000. Two installation defects that silently disabled pljava on WHPG19: Makefile: $GPHOME/etc/environment.d arrived in 7X and every later line keeps it, but installconfs tested GP_MAJORVERSION for equality with 7. WHPG19 reports 19, so 30-pljava.conf - and with it the JAVA_HOME and LD_LIBRARY_PATH setup the JVM needs - was never installed. gppkg v2 (used from 7X on) has no PostInstall of its own, so nothing else covered it. Gate on "7 or newer" instead. Makefile: uninstall removed 10-pljava.conf while install writes 30-pljava.conf, leaving the file behind. 30-pljava.conf: the jdk 8 libjvm path hardcoded jre/lib/amd64/server, which does not exist on aarch64. Derive the arch component instead. Also add the missing <math.h> to Time.c and Timestamp.c: floor() and rint() were implicitly declared, which GCC 14 and C23 reject outright.
…heck The 7/7 WHPG19 run PR #2 was signed off on was not reproducible: it needed packages installed into the container by hand, so nothing could gate it. ci/Dockerfile pins the toolchain into the image. It defaults to the shared build image and also produces a working image from a bare distro, so the package list is complete rather than a diff against one base. What the shared image ghcr.io/warehouse-pg/whpg-rocky9-build was actually missing is narrower than previously reported: maven and the JDK 11/17 devel packages are present, but JDK 8 is the headless runtime only - rt.jar and libjvm.so, no javac - and JDK 8 is the one concourse/pre_build.sh pins the native build to. JDK 21 is absent. Each remaining package is commented with why it is needed; the non-obvious ones are gcc-c++ (nar drives its gpp linker and fails with a bare NullPointerException without it), krb5-devel (the server headers are built --with-gssapi, so libpq/pg-gssapi.h pulls in gssapi.h), perl (WarehousePG pg_regress diffs through gpdiff.pl) and python3-psycopg2/psutil (gpstop and gpstart import them from the system interpreter, not from $GPHOME/lib/python). ci/run-regress.sh builds, installs and runs gpdb/tests, creating a demo cluster if none is running. It drops LD_LIBRARY_PATH and re-sources greenplum_path.sh before starting the cluster, so the JVM can only be found through the 30-pljava.conf that make install writes: a run where installconfs was skipped now fails instead of passing on a hand-exported path, which is how that gate came to be skipped on WHPG19 unnoticed. BUILD_JDK and RUNTIME_JDK are separate because only JDK 8 can build this tree - the top-level pom.xml compiles -source/-target 1.6 against $JAVA_HOME/jre/lib/rt.jar, and JDK 9+ has neither. The JVM the server loads is the axis that actually varies: 8, 11 and 17 pass; 21 needs pljava_vmoptions=-Djava.security.manager=allow and then differs only in CLDR locale formatting, so the lane runs it non-blocking. ci/mpp-signal-check.sh exercises the MPP-23735 SIGUSR1 substitution that PR #2 had to rewrite, by running a Java function across every segment and delivering SIGUSR1 to the segment backends while they are inside the JVM. Upstream has no MPP and therefore no coverage for it by construction. .github/workflows/whpg19-regress.yml ties this together. It needs a container image carrying an installed WHPG19, which is not published yet; without one the job stops with a clear error rather than reporting a green run that tested nothing. ci/README.md documents reproducing it locally.
My first cut made the regress job exit 1 when WHPG19_SERVER_IMAGE is unset, on the reasoning that a skipped run should not look green. On a pull_request trigger that is the wrong trade-off: no such image is published yet, so every PR goes red on a condition the author cannot fix, which just trains people to ignore the check. It reddened this PR itself. The job is now gated with an `if:` so it is skipped rather than failed, and a `scripts` job always runs so a PR still gets a real signal - shell syntax, the executable bits, and an assertion that installconfs still gates on "GP_MAJORVERSION >= 7" rather than reverting to an equality test against 7, which is the mistake that silently skipped 30-pljava.conf on WHPG19 in the first place.
Second ungated change from PR #2, and the one my PG12 audit missed. PR #2 deleted the SPI_restore_connection() call from SPI_rollbackSavepoint on the grounds that it is a documented no-op. That is true from PostgreSQL 10 on, which is why it is invisible on WarehousePG 7 and WHPG19 - PG12 has it as `#define SPI_restore_connection() ((void) 0)`. WarehousePG 6 is PG9.4-based, where it is still a real function that re-establishes the SPI connection after the subtransaction is aborted. Verified against the real headers: `extern void SPI_restore_connection(void)` in WHPG_6X_STABLE and in upstream through REL9_6_STABLE, the no-op macro from REL_10_STABLE, and gone entirely in REL_17_STABLE. Without the call, javatest.testTransactionRecovery() on WHPG6 fails with ERROR: java.sql.SQLException: SPI function SPI_prepare failed with error SPI_ERROR_UNCONNECTED and every later statement in the test aborts with "current transaction is aborted". Caught by smoke:pljava-el9-amd64 in the whpg-extensions-packaging WHPG6 lane, which is exactly the coverage a local PGXS build does not give. Gated on PG_VERSION_NUM < 100000 so it is called only where it does work and is not emitted on PG17+, where the symbol no longer exists.
#5 landed as a squash, so this branch was not an ancestor of whpg and its copy of TupleTable.c would have reverted the comment that merged - the narrower "WarehousePG 7 is PG12" wording, rather than naming both older lines. Only a comment, but it re-narrows exactly the misconception that caused the WHPG6 break, so the conflict is resolved to whpg on both gate files. What is left here is the CI work and the installation fixes; SPI.c and TupleTable.c are byte-identical to whpg.
Bonartze
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The build image
ci/Dockerfilepins the toolchain, so a WHPG19 run no longer depends onpackages installed by hand. What the shared build image
ghcr.io/warehouse-pg/whpg-rocky9-buildactually lacks is narrower than theearlier spike write-up claimed — maven and the JDK 11/17 devel packages are
already there:
rt.jarandlibjvm.soare present,javacis notJDK 8
javacis the one that matters:concourse/pre_build.shpins the nativebuild to JDK 8, and it is the only JDK that can build this tree.
The file defaults to the shared image and also produces a working image from a
bare distro (
--build-arg BASE_IMAGE=rockylinux:9), so the package list iscomplete rather than a diff against one base. Each addition is commented with
why; the non-obvious ones, all found by a build failing without them:
gcc-c++— nar drives the native build through itsgpplinker and failswith a bare
NullPointerExceptionthat names nothing.krb5-devel— the server is configured--with-gssapi, and that leaks intothe public headers:
libpq/pg-gssapi.hincludes<gssapi/gssapi.h>.perl— WarehousePG'spg_regressis not stock; it compares output throughgpdiff.pl, so without perl the suite cannot run at all.python3-psycopg2,python3-psutil—gpstop/gpstartimport these fromthe system interpreter, not
$GPHOME/lib/python.iputils,iproute—gpinitsystemshells out toping/ip/ssduringhost checks and exits FATAL without them.
The runner and the lane
ci/run-regress.shbuilds, installs and runsgpdb/tests, creating a democluster if none is running. It deliberately drops
LD_LIBRARY_PATHandre-sources
greenplum_path.shbefore starting the cluster, so the JVM can onlybe found through the
30-pljava.confthatmake installwrites. A run whereinstallconfswas skipped now fails instead of passing on a hand-exportedpath — which is exactly how that gate went unnoticed on WHPG19.
BUILD_JDKandRUNTIME_JDKare separate because only JDK 8 can build thistree: the top-level
pom.xmlcompiles-source/-target 1.6against$JAVA_HOME/jre/lib/rt.jar, and JDK 9+ has neither (11 fails with "Unable tofind package java.lang in classpath or bootclasspath", 17 and 21 with "Source
option 6 is no longer supported"). The JVM the server loads is the axis that
varies:
SecurityManageris disabled by default from JDK 18. Withpljava_vmoptions=-Djava.security.manager=allowthe only remaining diffs are CLDR locale formatting, so the lane runs it non-blocking.github/workflows/whpg19-regress.ymlruns that matrix. Theregressjob isskipped, not failed, when no WHPG19 server image is configured — none is
published yet, and failing would put a red X on every PR on a condition the
author cannot fix (it reddened this PR on its first revision). A
scriptsjobalways runs so a PR still gets a signal: shell syntax, exec bits, and an
assertion that
installconfskeeps gating onGP_MAJORVERSION >= 7ratherthan reverting to an equality test.
MPP coverage
ci/mpp-signal-check.shexercises the MPP-23735 SIGUSR1 substitution that #2had to rewrite —
pqsignal()no longer returns the previous handler, soPLCatchupInterruptHandlercallsprocsignal_sigusr1_handler()by name. Itruns a Java function across every segment and delivers SIGUSR1 to the segment
backends while they are inside the JVM. Upstream has no MPP and therefore no
coverage for this by construction.
Verified the hardcoded handler is the right one on the older lines too:
postgres.c:4874onWHPG_6X_STABLEinstalls exactlypqsignal(SIGUSR1, procsignal_sigusr1_handler).Installation fixes
These are WHPG19 defects, not regressions — #2 added WHPG19 support without
updating gates that predate it.
Makefile:installconfsgated onGP_MAJORVERSION == 7. WHPG19 reports19, somake installprinted "Nothing to be done for 'installconfs'" and30-pljava.conf— the file that tells the postmaster wherelibjvm.sois —was never installed. Nothing else covers it: gppkg v2, used from 7X on,
has no
PostInstallof its own; only the legacy v1 spec appends togreenplum_path.sh. Visible in the shipped artifact:This is also the real cause of the spike's "LD_LIBRARY_PATH/JAVA_HOME must be
in the postmaster's actual environment" note — read as an environment quirk,
actually a missing file. Now gates on "7 or newer"; GP6 is unchanged
(
6 -ge 7is false), which the WHPG6package:job confirms.Makefile:uninstallremoved10-pljava.confwhileinstallwrites30-pljava.conf, leaving the file behind.30-pljava.conf: hardcodedjre/lib/amd64/server, which does not existon aarch64.
Time.c,Timestamp.c:floor()andrint()with no#include <math.h>.Pre-existing, not from Adapt pljava-so native code for WHPG19 #2. GCC 11 warns; GCC 14 and C23 reject it.
Evidence
WHPG19 is not a packaging target, so it was checked locally against
/usr/local/whpg/bin/pg_config(PG19beta1, GP 19.0.0) on a 3-segment cluster:7/7, no
regression.diffs, with the JVM path coming only from the installedconf. Reproduced in a fresh container built solely from
ci/Dockerfile— nopackages installed by hand and no maven cache, the 924 downloads from central
being the proof nothing was pre-seeded.
Packaging CI for this branch, el9/amd64,
cloudsmith-upload=false— thesematter because the
Makefileand30-pljava.confchanges alter what lands inthe package on every line:
package:smoke: