Add support for Oracle connections string to support failover setups by DeepDiver1975 · Pull Request #35413 · owncloud/core

DeepDiver1975 · 2019-06-03T21:04:08Z

Description

Some failover/HA configurations in the Oracle world require support of connect strings.
Examples can be found here: https://docs.oracle.com/database/121/HABPT/config_fcf.htm#HABPT5391

This PR adds installation and operations using a new database config parameter: dbconnectionstring in config.php

Motivation and Context

Support Oracle HA/Failover scenarios

How Has This Been Tested?

basic connectivity is done manually and using drone
real fail over scenario is to be tested manually as well

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Database schema changes (next release will require increase of minor version instead of patch)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

Open tasks:

Backport (if applicable set "backport-request" label and remove when the backport was done)

patrickjahns · 2019-07-02T15:58:54Z

CI failed on this - lets restart it and see the results

DeepDiver1975 · 2019-07-02T18:34:54Z

Smb external failed. Known issue. Unrelated

codecov · 2019-07-03T00:50:50Z

Codecov Report

Merging #35413 into master will decrease coverage by 0.14%.
The diff coverage is 17.94%.

@@             Coverage Diff              @@
##             master   #35413      +/-   ##
============================================
- Coverage     65.81%   65.66%   -0.15%     
+ Complexity    18818    18667     -151     
============================================
  Files          1228     1221       -7     
  Lines         70982    70602     -380     
  Branches       1289     1288       -1     
============================================
- Hits          46716    46361     -355     
+ Misses        23888    23864      -24     
+ Partials        378      377       -1

Flag	Coverage Δ	Complexity Δ
#javascript	`53.69% <ø> (-0.01%)`	`0 <ø> (ø)`
#phpunit	`67.04% <17.94%> (-0.16%)`	`18667 <0> (-151)`

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Setup/OCI.php	`0% <0%> (ø)`	`35 <0> (-1)`	⬇️
lib/private/Setup/AbstractDatabase.php	`0% <0%> (ø)`	`12 <0> (ø)`	⬇️
core/Command/Maintenance/Install.php	`0% <0%> (ø)`	`21 <0> (+1)`	⬆️
lib/private/DB/ConnectionFactory.php	`90.27% <63.63%> (+0.42%)`	`21 <0> (+1)`	⬆️
apps/federatedfilesharing/lib/Notifications.php	`11.53% <0%> (-30.24%)`	`3% <0%> (-42%)`
apps/files_external/lib/config.php	`10.71% <0%> (-24.29%)`	`19% <0%> (+19%)`
apps/dav/lib/Connector/Sabre/Node.php	`75% <0%> (-6.15%)`	`49% <0%> (-1%)`
lib/private/DB/MigrationService.php	`85.29% <0%> (-5.15%)`	`53% <0%> (ø)`
apps/files_external/lib/Lib/Storage/SMB.php	`47.94% <0%> (-4.5%)`	`0% <0%> (ø)`
lib/private/Files/External/ConfigAdapter.php	`81.08% <0%> (-2.75%)`	`20% <0%> (+3%)`
... and 47 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 68fb368...3e764f0. Read the comment docs.

codecov · 2019-07-03T00:51:12Z

Codecov Report

Merging #35413 into master will decrease coverage by 1.38%.
The diff coverage is 23.07%.

@@             Coverage Diff              @@
##             master   #35413      +/-   ##
============================================
- Coverage     67.18%    65.8%   -1.39%     
- Complexity    18760    18762       +2     
============================================
  Files          1168     1229      +61     
  Lines         63575    70861    +7286     
  Branches          0     1289    +1289     
============================================
+ Hits          42716    46630    +3914     
- Misses        20859    23853    +2994     
- Partials          0      378     +378

Flag	Coverage Δ	Complexity Δ
#javascript	`53.7% <ø> (?)`	`0 <ø> (?)`
#phpunit	`67.19% <23.07%> (ø)`	`18762 <0> (+2)`	⬆️

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Setup/OCI.php	`0% <0%> (ø)`	`35 <0> (-1)`	⬇️
lib/private/Setup/AbstractDatabase.php	`0% <0%> (ø)`	`12 <0> (ø)`	⬇️
core/Command/Maintenance/Install.php	`0% <0%> (ø)`	`21 <0> (+1)`	⬆️
lib/private/DB/ConnectionFactory.php	`90.41% <100%> (+0.55%)`	`22 <0> (+2)`	⬆️
core/js/oc-dialogs.js	`2.66% <0%> (ø)`	`0% <0%> (?)`
core/js/oc-backbone.js	`50% <0%> (ø)`	`0% <0%> (?)`
settings/js/admin-apps.js	`6.46% <0%> (ø)`	`0% <0%> (?)`
core/js/config.js	`3.33% <0%> (ø)`	`0% <0%> (?)`
apps/files_sharing/js/external.js	`77.96% <0%> (ø)`	`0% <0%> (?)`
apps/comments/js/commentmodel.js	`100% <0%> (ø)`	`0% <0%> (?)`
... and 55 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 794f9b6...b645543. Read the comment docs.

phil-davis · 2019-07-03T00:51:22Z

A rebase gets this up-to-date with current drone that avoids the dead smb_windows - done.

DeepDiver1975 · 2019-07-15T07:00:07Z

 install_cmd+=' --database-connection-string=(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=oracle)(PORT=1521))(CONNECT_DATA=(SERVICE_NAME=XE)))                      --database-user=autotest                      --database-pass=owncloud'
+ php occ maintenance:install -vvv --database=oci --database-name=XE --database-table-prefix=oc_ --admin-user=admin --admin-pass=admin --data-dir=/drone/src/data '--database-connection-string=(DESCRIPTION=(ADDRESS=(PROTOCOL=TCP)(HOST=oracle)(PORT=1521))(CONNECT_DATA=(SERVICE_NAME=XE)))' --database-user=autotest --database-pass=owncloud
wait-for-oracle: oracle:1521 available after 1 seconds
Oracle username and/or password not valid
 -> You need to enter either an existing account or the administrator.

jvillafanez · 2019-07-26T07:01:05Z

I assume that all the DB parameters are required. There are a couple of things that aren't clear for me:

If I use the connection string, do I need to use the rest of the parameters?
In case that the connection string contains the username and password and maybe other parameters, what is the precedence of those against the one submitted as parameters in the command?

I think we should clarify both things in the command help

patrickjahns · 2019-07-26T08:24:00Z

 		}
-		if (!$result or $row[0]==0) {
+		// the connection is not needed anymore
+		\oci_close($connection);


What issue triggered the need for closing the connection here?

This was a long time dangling resource - we shall free any unused resources.

I discovered this while work on the code base

patrickjahns · 2019-07-26T08:30:41Z

Code looks fine to me

real fail over scenario is to be tested manually as well

Any recommendation for QA here, how to setup a failover scenario with oracle?
I haven't yet setup a oracle cluster with failovers - thinking of a simpler scenario to get started.
Could we do 2 oracle docker containers with both installed a owncloud database. Then just kill whichever is the primary and expect the app to still work ?

DeepDiver1975 · 2019-07-26T08:40:27Z

Any recommendation for QA here, how to setup a failover scenario with oracle?
I haven't yet setup a oracle cluster with failovers - thinking of a simpler scenario to get started.
Could we do 2 oracle docker containers with both installed a owncloud database. Then just kill whichever is the primary and expect the app to still work ?

I lack the knowledge on how to setup an oracle cluster - no idea if the XE dockers even can do this.

DeepDiver1975 · 2019-07-29T14:22:02Z

I think we should clarify both things in the command help

parameter description has bee adjusted - THX

patrickjahns

LGTM

DeepDiver1975 added 3 - To Review backport-request labels Jun 3, 2019

DeepDiver1975 requested a review from patrickjahns June 3, 2019 21:04

DeepDiver1975 self-assigned this Jun 3, 2019

phil-davis force-pushed the feature/oracle-connection-string branch from 19758ce to 3e764f0 Compare July 3, 2019 00:50

DeepDiver1975 force-pushed the feature/oracle-connection-string branch 2 times, most recently from 58a2b85 to 22e71c3 Compare July 9, 2019 06:49

DeepDiver1975 added 2 - Developing and removed 3 - To Review labels Jul 15, 2019

DeepDiver1975 force-pushed the feature/oracle-connection-string branch 2 times, most recently from 527fbcb to b645543 Compare July 25, 2019 11:42

DeepDiver1975 added 3 - To Review and removed 2 - Developing labels Jul 25, 2019

DeepDiver1975 changed the title ~~Add support for Oracle connections string too support failover setups~~ Add support for Oracle connections string to support failover setups Jul 25, 2019

DeepDiver1975 requested a review from jvillafanez July 25, 2019 14:28

patrickjahns reviewed Jul 26, 2019

View reviewed changes

Add support for Oracle connections string too support failover setups

bb5db2b

DeepDiver1975 force-pushed the feature/oracle-connection-string branch from b645543 to bb5db2b Compare July 29, 2019 13:57

patrickjahns self-requested a review July 29, 2019 15:06

patrickjahns approved these changes Jul 29, 2019

View reviewed changes

DeepDiver1975 merged commit f9b4b51 into master Jul 30, 2019

delete-merged-branch Bot deleted the feature/oracle-connection-string branch July 30, 2019 05:39

DeepDiver1975 mentioned this pull request Jul 30, 2019

Add support for Oracle connections string too support fail… #35945

Closed

DeepDiver1975 mentioned this pull request Nov 28, 2019

[For 10.4] Support Oracle connect string #36489

Merged

10 tasks

Conversation

DeepDiver1975 commented Jun 3, 2019

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Open tasks:

Uh oh!

patrickjahns commented Jul 2, 2019

Uh oh!

DeepDiver1975 commented Jul 2, 2019

Uh oh!

codecov Bot commented Jul 3, 2019

Codecov Report

Uh oh!

codecov Bot commented Jul 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

phil-davis commented Jul 3, 2019

Uh oh!

DeepDiver1975 commented Jul 15, 2019

Uh oh!

jvillafanez commented Jul 26, 2019

Uh oh!

patrickjahns Jul 26, 2019

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 Jul 26, 2019

Choose a reason for hiding this comment

Uh oh!

patrickjahns commented Jul 26, 2019

Uh oh!

DeepDiver1975 commented Jul 26, 2019

Uh oh!

DeepDiver1975 commented Jul 29, 2019

Uh oh!

patrickjahns left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Jul 3, 2019 •

edited

Loading