NH-4026 - Upgrade Firebird driver and use server #639

fredericDelaporte · 2017-06-02T17:45:39Z

NH-4026 - upgrading Firebird driver and using server instead of embedded mode.

…ded mode.

…ebird server

hazzik · 2017-06-02T23:32:19Z

I think we need to replace whole teamcity/lib folder with the NuGet sourced packages. It's not easy achievable, but it will allow greater manageability.

… look to need them.

fredericDelaporte · 2017-06-03T13:53:43Z

I think we need to replace whole teamcity/lib folder with the NuGet sourced packages. It's not easy achievable, but it will allow greater manageability.

Could be easily done if we just add them all bluntly to the test project, provided they do not conflict.

fredericDelaporte · 2017-06-03T14:45:40Z

Ok, I had forgotten about the TestDatabaseSetup which is run only on TeamCity.

…ly get it working.

hazzik · 2017-06-03T22:56:09Z

Could be easily done if we just add them all bluntly to the test project, provided they do not conflict.

No, it's not so easy. There is also ShowBuildMenu.bat and "available-test-configurations"/"current-test-configuration"

fredericDelaporte · 2017-06-04T09:42:08Z

It will just have to move around hibernate.cfg, and no more libs, if the test project have directly all references.

…ot only returned to pool. Clearing the pool as a solution.

This reverts commit ab0410c.

ngbrown · 2017-06-04T21:41:30Z

I had been working on why huge numbers of tests for Firebird were failing in #626. I had updated to the most recent Firebird NuGet package.

It was being caused by either

TearDown : NHibernate.HibernateException : unsuccessful metadata update
object INVERSED_STUFF is in use

or sometimes

TearDown : NHibernate.HibernateException : lock conflict on no wait transaction
unsuccessful metadata update
object HIBERNATE_UNIQUE_KEY is in use

It wasn't consistent which specific test would fail to tear down, but once it happened, it would start a massive chain reaction and fail lots of tests after that.

I had narrowed it down to a change in the driver from 2.7.7 to 3.0.0. It looks like you encountered the same and solved it with clearing the connection pool.

ngbrown · 2017-06-04T21:41:53Z

As for the embedded server, I don't see why not to continue to use it for unit tests... It's easier to setup that way, much like how SQLite is.

fredericDelaporte · 2017-06-04T22:02:17Z

I am doing that for testing DTC issues with as much as possible databases. I am not sure Firebird will behave the same about distributed transactions in embedded mode. True, due to a bunch of others Firebird issues, it will likely stay un-testable for DTC.

Moreover, SQLite is meant to be used only as embedded, but not Firebird. Firebird is a more classical database server, likely more frequently used in server mode than in embedded mode. I rather test its main use case than what is, I think, a "side" use case.

I have posted this question on Stack Overflow about the Firebird trouble with dropping table and connection pooling. If you have spotted the exact change having caused that, maybe could it be of interest you tell it in a comment.

Now there are still three newly failing tests (and two old failures), will probably check them tomorrow.

ngbrown · 2017-06-04T22:24:25Z

@fredericDelaporte I hadn't figured out the exact cause yet. It seems you are a bit ahead of me already having a solution. I had just narrowed it down to what version had the change. I have the suspicion it has something to do with this change DNET-337, but not really sure.

A note on your stack overflow question; the problems I'm testing against is that it's not actually the server version that breaks the tests, but the driver version that makes the difference. I've been testing with various versions of the driver, still with the same embedded server that's been part of the NHibernate repository.

hvlad · 2017-06-05T14:55:09Z

@ngbrown i'm not expert in .Net but it seems that change of wait mode (from "wait" to "nowait") by DNET-337 could lead to such behaviour in DDL transactions (if tests uses default transactions mode).
Workaround is easy (i think) - use explicit parameteres for (at least) DDL transaction.
Hope it helps

fredericDelaporte · 2017-06-05T15:22:17Z

@hvlad, maybe not that easy to do. I do not really understand what are those parameters you are referring too. They are likely Firebird concepts, but NHibernate schema creation/drop logic is database agnostic. Querying and transaction handling too. We are limited in what we can additionally do specifically for a database. Currently for this trouble we have this workaround which seems to do the job.

If there are some globally configurable parameters in the Firebird ADO.Net Data Provider, we may change them in its NHibernate driver encapsulation, but likely only provided such a change would be suitable for general purpose usage.

…er fb conn str.

fredericDelaporte · 2017-06-06T12:14:04Z

Now:

upgraded (driver from 2.5.2 to 5.9.1) + server (from embedded 2.5.2 + a (unused I think) 3.0.1 to a 3.0.2 server)
using server mode instead of embedded mode
no more additional libraries with driver
no more failing test at all
previous test result removed and check against previously failed removed

hazzik · 2017-06-06T12:26:46Z

src/NHibernate.Test/NHSpecificTest/NH3374/FixtureByCode.cs

 					{
 						m.Cascade(Mapping.ByCode.Cascade.All);
+						m.Column("`Blob`");


I think the correct way to do this is to mark "blob" as a keyword for Firebird dialect.

protected virtual void RegisterKeywords() { RegisterKeyword("date"); }

Maybe a lot more are missing...

And does not work anyway. Test failing again.

Those keywords are seldom used in the code base. Only the Template class uses them, and not for quoting.

Why "date"? it's "blob"

That is the current only registered word. For my local test I have added "blob". But that has no effect on column quoting.

fredericDelaporte · 2017-06-06T12:27:50Z

On Teamcity side, should the failure condition at least one test failed be enabled? I have compared with other builds not having test results comparison, only NHibernate build has it enabled. Sql Server 2012, SQLite and PostgreSQL do not have it enabled.

hazzik · 2017-06-06T12:28:27Z

teamcity.build

 			</fileset>
 		</copy>
-		<property name="NHibernate.Test.IgnoreFail" value="true" />


hazzik · 2017-06-06T22:44:07Z

@amroel can you also take a look?

ngbrown · 2017-06-06T22:51:52Z

For embedded mode, I think you are probably missing the ClientLibrary=fbclient.dll; part of the connection string.

I've gotten v3.0.2 working in embedded mode. See ngbrown-forks@c930471

It's currently down to 49 failures and taking 25 minutes (on my laptop).

fredericDelaporte · 2017-06-07T10:15:10Z

@ngbrown, I have tested with ClientLibrary=fbclient, not sure adding .dll will change anything. Without that param, the error is it tries to load embed instead, and I am not having that error. Maybe a specific trouble with my own local server. I will check your branch anyway, thanks.

On my own laptop in server mode, the test takes 40 minutes, so no that far from your 25 minutes in embedded mode: it could be just hardware differences. While it was taking 6 to 7 minutes on build agent with 2.5.2 version. Have you ascertain it was actually embedded? I test that by stopping Firebird service. In such case, in pseudo-embedded mode where the fbclient.dll just acts as a native client to the server, I get a meaningful error: Unable to complete network request to host "xnet://Global\FIREBIRD".. When I have the service up, I was having instead connection lost to database. But now I have found "why": my test runner had switch back to 64bits without me noticing... Now I get it working, and with Firebird service down, so very likely as actually embedded. Waiting to have some timings.

@hazzik, Yes a big timeout would not be a good thing. If we had to do that, we should then probably schedule such build only for running daily at most, for reducing its impact on build agent availability.

fredericDelaporte · 2017-06-07T10:47:55Z

Try some of these before increasing the limit: https://ib-aid.com/en/articles/45-ways-to-speed-up-firebird-database/

On my laptop, the bottleneck in embedded mode seems to be the disk (although it is a SSD, M.2 sata 3 port). So their parameters for increasing cache and reducing writes may help, if we can apply them in embedded mode. I will check them in server mode too.

It seems that in embedded mode, the provider has some differences in behavior, as Nathan fix for boolean in its branch seems to tell. This may require adding an embedded dialect...

fredericDelaporte · 2017-06-07T12:27:29Z

None of their parameters were having significant impact, excepted changing the database itself for disabling forced writes: duration drops to 7 minutes on my laptop. Committing that change. Forced writes are enabled by default since long in Firebird. I guess the old data provider was disabling it instead of following Firebird default.

Firebird advices strongly against disabling it. They consider its disabling as the major source of corrupted databases. Since we recreate it at each run, that is not an issue for us.

fredericDelaporte · 2017-06-07T15:35:20Z

Still 40 minutes. Trying some tweaking directly on server.

fredericDelaporte · 2017-06-07T16:37:49Z

15 minutes now: I have manually deleted the database. It looks like overwrite option is not able of changing forced writes. So for staying on the safe side, I will remove my simplification and go back to "try drop then create". I will try to gain even more performances server side by moving the db file to D:: it is supposed to be a local drive according to Azure documentation, while C: is in the network. (Such a move cannot be done for much other databases, because they will likely not support losing unexpectedly a database after a reboot, while Firebird does not care.)

@hazzik, I think we have an additional trouble on the build agent. Oracle take 1Gb of memory alone (counting Java in it, maybe I am wrong doing so, but well, then it takes 800Mb). The server has only 3.5Gb, so that is a bit problematic. On my laptop, using the same XE 11 I believe, it takes only 200Mb. I have already restarted the service on the agent, no better. Maybe some maintenance is needed on that Oracle database, but I do not know much about that.
Side note: SQL-Server 2012 was failing most tests with timeouts. Restarting its service seems to have solved that issue.

…ping rather than overwriting.

hazzik · 2017-06-07T21:12:54Z

lib/teamcity/firebird/firebird_installation.txt

@@ -8,7 +8,7 @@ Installation steps for Firebird for NH TeamCity:
 5.  Go into Firebird folder (c:\program files\firebird\) and create a folder named Data;
 6.  Go in Firebird installation directory and open databases.conf;
 7.  Add in "Live Databases" section:
-nhibernate = C:\Program Files\Firebird\Data\nhibernate.fdb
+nhibernate = D:\SqlData\Firebird\nhibernate.fdb


Why not make it local?

Local to the database server? Would be on C:, which is not local to the host. Local to the NHibernate test client? The path seems not frozen, only embedded mode can do that, where the client is the server too. I think Firebird mainstream usage is as an actual server, not as embedded mode. So I would rather test it as an actual server. And as showcase by Nathan branch, Firebird v3 embedded mode seems to need a new dialect.

When using a database server, does it really make sens to locate the database relatively to the client? That should be the server choice, not the client. Currently this is done with a configured alias in FireBird_3_0\databases.conf file, which decides where to put the file for the alias nhibernate.

hazzik · 2017-06-07T21:18:45Z

src/NHibernate.TestDatabaseSetup/TestDatabaseSetup.cs

@@ -102,8 +102,17 @@ private static void SetupSqlServerOdbc(Cfg.Configuration cfg)

 		private static void SetupFirebird(Cfg.Configuration cfg)
 		{
+			Directory.CreateDirectory(@"D:\SqlData\Firebird");


I don't like references to a specific folder. I think we need to use a local (relative) folder. Building with VS and NAnt+MSBuild will produce different results. When building with NAnt the output folder would be /build/version/net40 (or something like this). Building with VS will produce tests assemblies in different folders (/src/NHibernate.Test/bin/Debug-2.0/, /src/NHibernate.TestDatabaseSetup/bin/Debug-2.0/, etc). We need to choose some common folder, probably "curent-test-configuration", or a repository root.

I think we shall/should parse connection string to find where the file sits.

The connection string does currently only hold the alias nhibernate. The actual path is in databases.conf file, in Firebird installation directory. Yes this CreateDirectory code line is a crutch for accounting the possible loss of D: content. The server do not create the directory itself. We may instead just drop the db on the root, D:\, avoiding this above line of code.

When using a database server, it does not really make sens for me to locate the database relatively to the client, that is usually the server choice. Firebird allows to specify the database location from the connection string, which is a strange model for a server, not at all common. So we may do that anyway, by detecting where the test assemblies are then tweaking the connection string for telling the server where to put the file. But that would depart from what is and can be done for other actual servers like Oracle, Sql-Server, PostgreSql, which gives no option to the client about where to locate the database.
And we would have to do that both in TestDatabaseSetup and the test project.

Otherwise we should go back to testing Firebird in embedded mode, with all its additional binaries and apparently some new behaviors in v3 requiring a dedicated dialect. (There are seven hundred failing tests when running in embedded mode with Firebird v3, most solved by redefining the boolean type as have done Nathan. I have not yet investigated the other cases, since I do not consider we should test Firebird as an embedded server.)

The problem, that people might not necessarily have a "D:" drive, and this would fail. I usually use TestDb to create a fresh copy of a database (including Firebird) myself.

So I may just locate the file on d:\ root if that prove to be any better than putting it within Firebird installation folder. That would avoid that line. Actually I was seeing TestDatabaseSetup as a Teamcity exclusive thing, managing myself my own local db. So I would avoid to do such changes now.

It still be executed if you run from ShowBuildMenu (release option, for ex)

Ok, I have missed that, sorry.

hazzik · 2017-06-07T21:21:13Z

On my laptop, using the same XE 11 I believe, it takes only 200Mb.

It's 12c

fredericDelaporte · 2017-06-08T10:26:39Z

It's 12c

The test settings for Oracle are using the 10g dialect, while a 12c one is available. That why I was believing it was the free XE 11 edition.

fredericDelaporte · 2017-06-08T10:30:22Z

About Firebird, locating that db on D: seems worst than on C:, although the temporary drive is supposed to be more performant than the system drive accoding to msdn:

This temporary storage drive is present on the physical machine which is hosting your VM and hence can have higher IOPS and lower latency when compared to the persistent storage like data disk.

The current run has taken 34 minutes to complete. Going to launch a new one in case that was some temporary degradation.

fredericDelaporte · 2017-06-08T10:43:54Z

Last run on D:, 7'36'', now launching another run on C: (best time was 15' for C:).

fredericDelaporte · 2017-06-08T11:10:24Z

Checking the 34' run on D:, publishing artifacts had taken 22' alone, so that was mostly unrelated to Firebird. Current run waiting the merge builds I have triggered from NH-4013. It seems you have just taken the session I was having on the build agent.

It has taken 16' on C:, re-running. Next run 16' on tests alone, then more than 30' on publishing. Reswitching to D:, but on root, without folder creation in TestDatabaseSetup.

hazzik · 2017-06-08T11:17:43Z

You can take the session back:)

fredericDelaporte · 2017-06-09T16:10:42Z

IMO, this is now ready for squashing&merging.

hazzik · 2017-06-10T12:40:39Z

Merged

fredericDelaporte · 2017-06-10T14:16:12Z

Thanks Alexander.
I have just disconnected (instead of log-off) from a session on the build agent where a PostgreSql install was finished. I have left it as is.

fredericDelaporte added 2 commits June 2, 2017 19:44

NH-4026 - upgrading Firebird driver and using server instead of embed…

7c8aa5a

…ded mode.

NH-4026 - A connection string parameter is missing for addressing Fir…

4d36a19

…ebird server

NH-4026 - some other changes just to try, although my laptop does not…

fd84733

… look to need them.

fredericDelaporte added 3 commits June 3, 2017 16:48

NH-4026 - fix the test database setup (or at least tries to).

fe0e8ac

NH-4026 - let the TestDatabaseSetup create the db.

728ede7

NH-4026 - trying to document the relevant things I have done to final…

704c122

…ly get it working.

fredericDelaporte force-pushed the NH-4026 branch from 6dac072 to 73198c2 Compare June 4, 2017 11:31

NH-4026 - Firebird locks table till connection are actually closed, n…

96ac0d3

…ot only returned to pool. Clearing the pool as a solution.

fredericDelaporte force-pushed the NH-4026 branch from 73198c2 to 96ac0d3 Compare June 4, 2017 18:05

fredericDelaporte added 3 commits June 4, 2017 21:13

NH-4026 - some more explanations.

ab0410c

Revert "NH-4026 - some more explanations."

da88449

This reverts commit ab0410c.

NH-4026 - some more explanations.

d75f7ef

fredericDelaporte added 3 commits June 5, 2017 19:04

NH-4026 - fixing remaining failing tests.

8b832cf

NH-4026 - remove ignoring previously failed.

e794efe

NH-4026 - fix a typo in default test config file. Align naming on oth…

d04fafa

…er fb conn str.

hazzik reviewed Jun 6, 2017

View reviewed changes

teamcity.build

</fileset>

</copy>

<property name="NHibernate.Test.IgnoreFail" value="true" />

Copy link

Member

hazzik Jun 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

NH-4026 - disabling forced writes and simplifying setup.

3a5395e

NH-4026 - Updating instruction file

d71f510

NH-4026 - relocating the db, and ensuring forced write option by drop…

b42cfb3

…ping rather than overwriting.

hazzik reviewed Jun 7, 2017

View reviewed changes

NH-4026 - removing explicit dependency on D: from TestDatabaseSetup.

907b5cf

hazzik merged commit ad4c2ef into nhibernate:master Jun 10, 2017

hazzik changed the title ~~NH-4026 - upgrade Firebird driver and use server~~ NH-4026 - Upgrade Firebird driver and use server Jun 10, 2017

fredericDelaporte deleted the NH-4026 branch June 10, 2017 14:16

fredericDelaporte added this to the 5.0.0 milestone Jun 10, 2017

hazzik added the r: Fixed label Aug 3, 2017

fredericDelaporte added the t: Improvement label Sep 6, 2017

fredericDelaporte mentioned this pull request Nov 7, 2017

SequenceStyleGenerator generates Ids twice on SQLite #1429

Closed

fredericDelaporte added the c: Tests label Aug 4, 2019

NH-4026 - Upgrade Firebird driver and use server #639

NH-4026 - Upgrade Firebird driver and use server #639

Uh oh!

Conversation

fredericDelaporte commented Jun 2, 2017

Uh oh!

hazzik commented Jun 2, 2017

Uh oh!

fredericDelaporte commented Jun 3, 2017

Uh oh!

fredericDelaporte commented Jun 3, 2017

Uh oh!

hazzik commented Jun 3, 2017

Uh oh!

fredericDelaporte commented Jun 4, 2017

Uh oh!

ngbrown commented Jun 4, 2017

Uh oh!

ngbrown commented Jun 4, 2017

Uh oh!

fredericDelaporte commented Jun 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ngbrown commented Jun 4, 2017

Uh oh!

hvlad commented Jun 5, 2017

Uh oh!

fredericDelaporte commented Jun 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredericDelaporte commented Jun 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte Jun 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte commented Jun 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hazzik commented Jun 6, 2017

Uh oh!

ngbrown commented Jun 6, 2017

Uh oh!

fredericDelaporte commented Jun 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredericDelaporte commented Jun 7, 2017

Uh oh!

fredericDelaporte commented Jun 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredericDelaporte commented Jun 7, 2017

Uh oh!

fredericDelaporte commented Jun 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte Jun 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredericDelaporte Jun 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

fredericDelaporte commented Jun 4, 2017 •

edited

Loading

fredericDelaporte commented Jun 5, 2017 •

edited

Loading

fredericDelaporte commented Jun 6, 2017 •

edited

Loading

fredericDelaporte Jun 6, 2017 •

edited

Loading

fredericDelaporte commented Jun 6, 2017 •

edited

Loading

fredericDelaporte commented Jun 7, 2017 •

edited

Loading

fredericDelaporte commented Jun 7, 2017 •

edited

Loading

fredericDelaporte commented Jun 7, 2017 •

edited

Loading

fredericDelaporte Jun 8, 2017 •

edited

Loading

fredericDelaporte Jun 8, 2017 •

edited

Loading

fredericDelaporte commented Jun 8, 2017 •

edited

Loading

fredericDelaporte commented Jun 10, 2017 •

edited

Loading