fix(db): speed up Oracle schema introspection and run the PHPUnit DB suite against Oracle again [10.16] - #41815
fix(db): speed up Oracle schema introspection and run the PHPUnit DB suite against Oracle again [10.16]#41815oc-tmueller wants to merge 3 commits into
Conversation
e98f70f to
a87f0ab
Compare
|
https://github.com/owncloud/core/actions/runs/34099036638/job/101669078066?pr=41815 Got the codeberg problem here in 10.16 still. |
5a355a3 to
2971b47
Compare
f2a8485 to
75b0b29
Compare
75b0b29 to
3566f5d
Compare
| run: | | ||
| make install-composer-deps | ||
|
|
||
| - name: TEMPORARY DEBUG - Oracle client |
There was a problem hiding this comment.
do we need all the debug stuff?
There was a problem hiding this comment.
this needs to go - thank you
There was a problem hiding this comment.
No. Removed in ca4a63c.
That step existed to answer one question: whether the hour-long maintenance:install was oci8 / the instant client / the TCP connection on PHP 7.4. It was none of those — it was doctrine/dbal 2.13 introspecting the schema per table, fixed by #41819, which is now in this branch. Install is back under a minute, so the probe has nothing left to tell us.
| php occ ${install_cmd} | ||
| if [[ "${DB_TYPE}" == "oci" ]]; then | ||
| export DSN="(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=${DB_HOST})(PORT=1521))(CONNECT_DATA=(SERVICE_NAME=${ORACLE_SERVICE_NAME})))" | ||
| cat > /tmp/oracle-watch.php <<'WATCHEOF' |
There was a problem hiding this comment.
and all this stuff, is it needed?
There was a problem hiding this comment.
The v$session / v$sql sampler is gone too (ca4a63c) — with it the background job, the 90 minute kill timeout and the top-SQL dump. Installing on Oracle is a plain php occ ${install_cmd} again, same as every other database.
What is left in this block is not debug and is needed: the --database-connection-string=…SERVICE_NAME=FREEPDB1… branch. gvenzl/oracle-free creates the app user inside the FREEPDB1 pluggable database, so host + database name would target the CDB root and the install fails. That is why --database-host is deliberately not passed there.
Oracle was never covered by the GitHub Actions workflows: in drone it ran only in the nightly cron matrix, and that pipeline is not part of the GitHub Actions setup. Add it as a php-unit.yml matrix entry driven by a new databases input, served by gvenzl/oracle-free:23-slim-faststart. Oracle lives in the FREEPDB1 pluggable database, so the installer is called with a connect string carrying SERVICE_NAME instead of a host. The oci8 extension is only requested for Oracle jobs, and DB_TYPE is only exported for Oracle so test-phpunit.sh narrows the run to --group DB - the other databases keep running the full suite unchanged. The Oracle job runs on every pull request and shares its display name with the existing php-unit job, so all databases appear in one group of checks. It remains a separate job so that a slow or failing Oracle run cannot cancel the other databases through fail-fast. Backport of #41808 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
…0.16] (#41819) * fix: avoid a deprecation notice when hashing the file cache path on Oracle Oracle cannot store empty strings, so the file cache converts them to null before writing a row. For the storage root, whose path is the empty string, that left md5() being called with null: PHP 8 reports that as a deprecated implicit null to string conversion, which is noise in the log and an error under PHPUnit's strict error handling. The stored path_hash itself was never wrong, because md5(null) coerces to md5(''). Found by running the PHPUnit DB suite against Oracle. Backport of #41808 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * tmp: debug oracle client on php 7.4 To be dropped before merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * fix(composer): consume icewind deps from GitHub mirrors to survive Codeberg outages [10.16] (#41816) fix(composer): consume icewind deps from GitHub mirrors to survive Codeberg outages Backport of #41707 to 10.16. owncloud CI intermittently fails because Composer must git-clone icewind/streams and icewind/smb from codeberg.org (no dist archive on Packagist), and Codeberg regularly returns HTTP 503/504. Every job that installs Composer dependencies is affected, including "Make local core" in downstream app builds. Add VCS repositories pointing at the GitHub mirrors so Composer fetches a dist zipball via the GitHub API and never touches Codeberg at install time. Unlike master, 10.16 stays on icewind/smb v3.7.0: the 3.8.x line requires php >= 8.2 while this branch pins platform php 7.4. Both packages are re-locked at the exact refs already in the lock (smb e6904cb, streams cb2bd3e), which the mirrors carry, so this is a pure source repoint with no version change. No codeberg.org references remain in composer.lock. Mirrors are kept in sync weekly by DeepDiver1975/codeberg-mirrors. Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix(db): introspect the Oracle schema in a fixed number of queries [10.16] doctrine/dbal 2.13 describes a schema table by table: for every table it runs one query for the columns, one for the indexes, one for the foreign keys and one for the table comment. Every one of those inlines the table name as a literal, so Oracle cannot share cursors and hard parses each of them at a cost of a few hundred milliseconds. OC\DB\Migrator asks for the full schema once per applied migration, which multiplies that per-table cost by the number of migrations. Add an OracleSchemaManager that reads the whole data dictionary in a fixed number of queries and use it for Oracle connections. Reading a 48 table schema drops from 194 to 6 queries, and occ maintenance:install against Oracle on PHP 7.4 drops from 43 minutes to 30 seconds. doctrine/dbal itself works this way from 3.4 onwards, so master is unaffected; its 3.x line cannot be pulled into 10.16 because it changes public API that third-party apps rely on. The introspected schema is unchanged. The added test asserts that the batched result is identical to the stock one according to Doctrine's Comparator, and that the query count does not grow with the number of tables. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> --------- Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The Oracle job carried two pieces of throwaway diagnostics that were only there to find out why maintenance:install took an hour on this branch: a "TEMPORARY DEBUG - Oracle client" step probing oci8, the instant client and a raw connect, and a v$session/v$sql sampler that wrapped the install in a background job with a 90 minute kill timeout and printed the top SQL afterwards. That question is answered - it was doctrine/dbal 2.13's per table schema introspection, fixed by #41819 - and the install is back to under a minute, so the instrumentation has no reason to stay. Installing on Oracle is a plain "php occ ${install_cmd}" again, like every other database. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
ca4a63c to
ccee112
Compare
Backport of #41808 to
10.16, plus the Oracle fixes needed to make the job usable on this branch.What is in this PR
1.
ci:the Oracle PHPUnit job —php-unit.ymlgains adatabasesinput (defaulting to today's five combinations, so the existing matrix is unchanged) and anoracleservice ongvenzl/oracle-free:23-slim-faststart, using the same "empty image string disables the service" idiom as themysql/postgresservices. The app user lives in theFREEPDB1pluggable database, so--database-hostis not passed and a connect string carryingSERVICE_NAMEis used instead.oci8is requested fromsetup-phponly for Oracle jobs, andDB_TYPEis exported only for Oracle sotest-phpunit.shnarrows the run to--group DB.ci.ymlcalls the reusable workflow a second time in a job whose display name is alsoPHP Unit, so all six databases appear in one group of checks. It is a separate job rather than a sixth matrix entry because this branch hasfail-fast: true— a slow or failing Oracle run must not cancel the other databases.No
nightly.ymlhere, unlike onmaster: GitHub only runsschedule:from the default branch, so a nightly workflow on a maintenance branch would never fire.2.
fix(db):batched Oracle schema introspection (OC\DB\OracleSchemaManager, wired in throughConnection::getSchemaManager()). Without this the job is not viable:occ maintenance:install --database=ocineeded over 40 minutes on this branch. doctrine/dbal 2.13 implementslistTables()aslistTableDetails()per table — four dictionary queries each, with the table name inlined as a literal, so Oracle hard parses every one — andMigrator::getDiff()introspects the whole schema once per applied migration. The subclass reads each category in one batched query over theuser_*views and feeds the rows to the very same_getPortableTable*()methods, so the resultingTableobjects are unchanged;tests/lib/DB/OracleSchemaManagerTest.phpasserts that equivalence. Introspecting a 48-table schema drops from 194 queries to 6 and the install from 43 minutes to ~30 seconds. dbal does this natively from 3.4 on, which is whymasternever needed it and why this file is10.16-only.3.
fix:themd5(null)cast inFiles\Cache\Cache— thefix:commit of #41808. Kept here so both branches hash the storage root the same way and the code does not depend on PHP 7.4 tolerating an implicitnull-to-string conversion; the storedpath_hashwas never wrong, becausemd5(null)coerces tomd5('').Not needed here: the
feat:commit of #41808, which restoresOC\Setup\OCI, thedbSetupClassesentry and--database-connection-string. On10.16none of that was ever removed — 95e3cde landed onmasteronly — so this branch can already be installed against Oracle from both the CLI and the web installer.Verification
Green on this branch. The Oracle job was run three times to see how stable the runtime is:
masterOracleoci8)maintenance:install --database=ociphpunit --group DBRunner variance dominates: in run #3 every database was slower, and in run #1 the pre-existing
mariadb:10.11job took 22 m 27 s while Oracle took 18 m 14 s. Side by side, run #3 was sqlite 8 m 41 s, mariadb:10.11 9 m 11 s, postgres:10.21 11 m 23 s, mariadb:10.6 12 m 24 s, mysql:8.0 16 m 58 s, Oracle 23 m 24 s. So Oracle is at the upper end of what this branch already runs but not out of line with it, which is why it is wired into every PR rather than gated behind afull-cititle. Note also that Oracle only runs--group DB, whereas the other databases run the full suite.PHPUnit passes:
Tests: 5412, Assertions: 35280, Skipped: 17, no failures or errors. The skips are the pre-existing platform skips, including the Oracle ones already in the tree.Two things worth recording, since neither could be predicted before the first run:
setup-phpresolvesoci8for PHP 7.4 in ~20 s (oci8 2.2.0 against Instant Client 23.26) — no source build, so no extension cache is needed. Same as onmaster, which gets oci8 3.4.0.OracleMigrator; no fallback togvenzl/oracle-xe:21was needed.Coverage is not a factor:
test-phpunit.shpasses--coverage-cloverunconditionally, but no coverage driver is installed, so PHPUnit only warns — exactly as it already does for the other databases here.Types of changes
Checklist
🤖 Generated with Claude Code