Giter Club home page Giter Club logo

Comments (57)

nakagami avatar nakagami commented on July 19, 2024 1

Hmmm.
Maybe it would be better not to start a transaction in connect().
Even if not, maybe it would be better not to start a transaction in connect() alone so as not to start a transaction unnecessarily.

In that case, efirebirdsql needs to be modified as well.
I'll give it a try, so please give me a few more days.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024 1

Hmmm. It was not what I expected.
Aren't you limited to a maximum number of connection poolings?

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Could it have something to do that you are using efirebirdsql_op:op_commit_retaining in efirebirdsql_protocol?

From: https://firebirdsql.org/file/documentation/chunk/en/refdocs/fblangref40/fblangref40-transacs.html
12.1.2 COMMIT
The COMMIT statement commits all work carried out in the context of this transaction (inserts, updates, deletes, selects, execution of procedures). New record versions become available to other transactions and, --> unless the RETAIN clause is employed, all server resources allocated to its work are released <--.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Thank you for the graph.

Perhaps calling commit_retaining will commit the transaction before ping() is executed.
Are you sure that the number of transactions is increasing at the point where ping() is called?

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

On this test database, nothing else was happening, only the ping. On a database with more activity the graph is more irregular but basically with the same pattern.
I was more thinking that a commit_retaining does not release all server resources (as in the specification in my previous post), while maybe a 'normal' commit would?

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

It may be impossible to implement ping by trying to execute a query

firebird3 allows pinging without attempting a query transaction.
Would a fix to PR #8 improve the situation?

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I'm getting errors when I try PR #8. Is it possible :efirebirdsql_protocol.ping does not work on a 2.5 database?

10:04:23.788 [error] GenServer #PID<0.323.0> terminating
** (MatchError) no match of right hand side value: {:error, :closed}
(efirebirdsql 0.8.4) /elixir/firebirdex/deps/efirebirdsql/src/efirebirdsql_op.erl:486: :efirebirdsql_op.get_response/1
(efirebirdsql 0.8.4) /elixir/firebirdex/deps/efirebirdsql/src/efirebirdsql_protocol.erl:274: :efirebirdsql_protocol.ping/1
(firebirdex 0.3.2) lib/firebirdex/connection.ex:38: Firebirdex.Connection.ping/1
(db_connection 2.4.2) lib/db_connection/connection.ex:184: DBConnection.Connection.handle_cast/2
(connection 1.1.0) lib/connection.ex:810: Connection.handle_async/3
(stdlib 3.17) gen_server.erl:695: :gen_server.try_dispatch/4
(stdlib 3.17) gen_server.erl:771: :gen_server.handle_msg/6
(stdlib 3.17) proc_lib.erl:236: :proc_lib.wake_up/3
Last message: {:"$gen_cast", {:ping, #Reference<0.161675571.234225666.162281>, %Firebirdex.Connection{conn: {:conn, #Port<0.11>, 'SYSDBA', '---PASSWORD---', true, 121515445435030628414104713960911197516, 39676290785175426407193449202719918328536398499309895854245515748377888796247630053710684217517483588665171718113727422125520234160354867780264259185558131096078006202173732444007737501619414512880179967484758523014428049301801093869705493840031519981511917761332135125922323765833901349864439372050087395726, :undefined, 'Srp256', true, :undefined, :undefined, 0, 1, 12, 'Europe/Brussels', %{}, %{}}}}}
State: {Firebirdex.Connection, %Firebirdex.Connection{conn: {:conn, #Port<0.11>, 'SYSDBA', '---PASSWORD---', true, 121515445435030628414104713960911197516, 39676290785175426407193449202719918328536398499309895854245515748377888796247630053710684217517483588665171718113727422125520234160354867780264259185558131096078006202173732444007737501619414512880179967484758523014428049301801093869705493840031519981511917761332135125922323765833901349864439372050087395726, :undefined, 'Srp256', true, :undefined, :undefined, 0, 1, 12, 'Europe/Brussels', %{}, %{}}}}

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Yes, yes
It does not work on Firebird 2.5.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I see, in that case it is not a solution in my case. The database is from an external company and I cannot upgrade to fb3.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Umm, I can't think of a good solution, now.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I will do some more investigations on it. Please give me a bit of time.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I have a few questions:

  1. Is it so that unless there is auto_commit: fase connection option, efirebirdsql defaults to true? (as specified in efirebirdsql_protocol:connect)
  2. Looking a bit further in efirebirdsql_protocol:connect there is this piece of code:
    case begin_transaction(AutoCommit, C2) of {ok, C3} -> load_timezone_data(C3); {error, ErrNo, Reason, C3} -> {error, ErrNo, Reason, C3} end;
    It starts a transaction for sure, but how does the database know when the transactions has finished? The case statement is an erlang construct, and in my opinion the database does not know when to commit the transaction, even when it is an auto_commit.

When I look at the close function I can see you are doing an explicit commit there. I think this is causing the number of pending transactions falling to 0 when I periodically close all the connection (as demonstrated in the picture in the first post).
I have a suspicion, but I'm not sure, that already the piece of code I included is opening a transaction, but never committing the transaction, and this responsible for continuously increasing the number of pending transactions on every new transaction.

What do you think?

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

In the case of autocommit, I think it is automatically committed when executing because isc_tpb_autocommit(=16) is added in begin_transaction().

https://github.com/nakagami/efirebirdsql/blob/master/src/efirebirdsql_protocol.erl#L169

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

I noticed that the driver can check the connection with allocate statement without executing the SQL statement,
so wouldn't the following modification improve the situation?

index 14d1dfe..a370155 100644
--- a/lib/firebirdex/connection.ex
+++ b/lib/firebirdex/connection.ex
@@ -39,8 +39,6 @@ defmodule Firebirdex.Connection do
   def ping( %__MODULE__{conn: conn} = state) do
     sql = "SELECT rdb$get_context('SYSTEM', 'ENGINE_VERSION') from rdb$database"
     with {:ok,statement} <- :efirebirdsql_protocol.allocate_statement(conn),
-      {:ok,statement} <- :efirebirdsql_protocol.prepare_statement(sql, conn, statement),
-      {:ok,_} <- :efirebirdsql_protocol.execute(conn, statement),
       {:ok,_} <- :efirebirdsql_protocol.free_statement(conn,statement,:drop)
     do
       {:ok,state}

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I tried your suggestion above, because at least it is a good performance improvement :-)
However I still see the pending transactions going up, but things get stranger. I reverted the ping function to just returning {:ok, state}, and still the pending transactions are going up!!??
Is there anything else inside Firebirdex or efirebirdsql that is going through an infinite loop?

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Another observation which might help:
image
When I start the Firebirdex with a pool_size of 20 you see a jump in pending transactions to 20, and then gradually it goes further up, until I disconnect.
When I next start Firebirdex with a pool_size of 50, the number of pending transactions immediately jumps to 50, and then again gradually increases.
This gives me the impression a transaction is started when a connection is opened which does not get committed. Is that possible?

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

One more observation (I'm trying to give you as much information as possible)
The rate of increase (slope of the curve) of pending transactions, stays the same wether the pool_size is 1 or 50. There is only a difference in the initial jump on connecting.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I found an interesting discussion about transactions in Firebirdsql, especially the answer from Adriano Carneiro: https://stackoverflow.com/questions/6256678/how-do-i-use-transactions-in-firebird

If I understand it correctly, just by opening a connection you are already in a transaction and that would explain the jump in transactions I see when increasing the pool_size as shown above.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Thanks a lot

In efirebirdsql, transactions are begun at the following two locations

We may need to do either of the following

  1. Do not begin transaction in efirebird_protocol.connect()
  2. Stop implementing handle_begin() https://github.com/nakagami/firebirdex/blob/master/lib/firebirdex/connection.ex#L133

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Looking at handle_begin() in PostgreSQL and MySQL
opts parameter to determine whether to start a transaction or not.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Will this fix solve the problem? #14
I will merge this pull request even if it does not affect this issue

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I will test your PR in my environment as soon as possible.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Still the same behaviour of pending transactions gradually increasing. Did you also remove the begin_transaction in the connect in efirebirdsql?

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

No.

But in connect(), you set :transaction_status to :transaction

https://github.com/nakagami/firebirdex/pull/14/files#diff-6f84ac9dc64cfc1c282f8069149b52ea7a7ebab7e82b9554a79a2dfe7b0bba52R23

And in handle_begin().
https://github.com/nakagami/firebirdex/pull/14/files#diff-6f84ac9dc64cfc1c282f8069149b52ea7a7ebab7e82b9554a79a2dfe7b0bba52R134
I am trying to avoid executing efirebirdsql_protocol.begin_transaction unnecessarily.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

#14
was merged because I thought it would be a good idea to merge it, even if this issue is not resolved

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

In this file https://github.com/nakagami/firebirdex/blob/master/lib/firebirdex/connection.ex
Somewhere in the function defined as @impl true
Can you find out what is being called by logging it out or something?

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Probably I don't completely understand. This is my understanding, and please correct me if I'm wrong:
By doing a begin_transaction in efirefirdsql during connect, a transaction is started, right?
Setting :transaction_status to :transaction in Firebirdex connect() says we are indeed in a transaction after the connect(), and the handle_begin() will now always set a save_point, and will not really start a new transaction. (I thought save_points are not intended to start/stop transactions, but give developers a possibility to rollback to a certain save_point within in transaction.)

My thinking is that efirebirdsql connect() should not start a transaction, and the :transaction_status in Firebirdex should then be set to :idle.
handle_begin() can then set the :transaction_status to :transaction whenever a new transaction is started, and also use a save_point within a transaction.

Also the ping() does not do a begin_transaction() , and still the number of pending_transactions is increasing, which looks like every ping increases the pending transactions, because a transaction is started at connect, and never committed, until close.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

The connect in MySQL for example also sets the transaction_status to :idle, and does not seem to begin a transaction:
https://github.com/elixir-ecto/myxql/blob/master/lib/myxql/connection.ex#L15

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Does this solve the problem? #15

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I'm sorry, it does not.

Looking at the code, it looks like you just moved the begin_transaction from connect() in efirebirdsql, to connect() in Firebirdex, but there is still a transaction started in connect(), and pending transactions are still increasing.

I noticed that when doing a begin_transaction the trans_handle in the conn structure changes from :undefined to 1, and from then on a transaction is open (and increasing the pending transactions with every new execute).

I changed the connect() in Firebirdex to not do a begin_transaction, and put a hardcoded 0 in trans_handle in the conn structure.
The effect seems to be that no transaction is started, and now of course every query must be wrapped in a transaction.
I changed ping() for example to:
with {:ok,conn} <- :efirebirdsql_protocol.begin_transaction(false,conn), {:ok,statement} <- :efirebirdsql_protocol.allocate_statement(conn), {:ok,statement} <- :efirebirdsql_protocol.prepare_statement(sql, conn, statement), {:ok,_} <- :efirebirdsql_protocol.execute(conn, statement), {:ok,_} <- :efirebirdsql_protocol.free_statement(conn,statement,:drop), :ok <- :efirebirdsql_protocol.commit(conn)

The effect in the monitoring tool is now this:
image

What you see is that on every ping the number of pending transactions jumps to 10 (because of pool_size=10), and after the commit they drop again, and this is what we want I think.

The only problem now of course is that we have to use a transaction on every single database request, so there is no auto commit anymore.

It looks like we have to find a way to open the connection in auto_commit mode without actually starting an initial transaction. I don't know enough of the Firebirdsql wire protocol wether that's possible.

If this is not possible I suggest an auto_commit connection option, which can default to true and then you do the connect() exactly as it is now in the fix_transaction branch, with the risk of pending transactions increasing. However, if someone sets the auto_commit connection option to false, we don't do a begin_transaction and set the trans_handle to 0, and it then it is the responsibility of the user of Firebirdex to properly wrap all database interactions in transactions.

Another option, when auto_commit = true, is also to do a begin_transaction and commit within Firebirdex on every execute, but that of course is more complex, more work, and would require also more testing.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Thanks a lot for your help.

I have organized transaction processing in efirebirdsql 0.9.0.

Within connect() efirebirdsql_protocol.begin_transaction() is executed, but it was not going to increase more than the number of connection pools.
Will there be more and more transactions as before?
Will the number of transactions decrease when I close a connection or when I commit and rollback?

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I'm not using efirebirdsql directly, I am using Firebirdex.
And since there is still a begin_transaction in the connect() of Firebirdex, the problem with increasing transactions is still happening in Firebirdex.
Do you want me to test efirebirdsql? (I will have to implement some kind of pooling & ping then efirebridsql to simulate what I'm doing in Firebirdex)

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

No
I wanted to know if using firebirdex would increase transactions.

I will try to figure out how to modify connect() so that it does not initiate a transaction.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

ah, ok, I misunderstood your question.
So, yes, transactions were still increasing with the latest Firebirdex fix_transaction branch.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Additional corrections made. 41386e7 on #15

  • Connect() returns a connection that has not started a transaction
  • ping() checks for a connection with allocate_statement() and does not execute a query

How about this?

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Unfortunately, it does not solve the issue.

When I now start my application using Firebirdex with a pool_size=10, initially pending transactions are not increasing with the pings, which is great.
But as soon as I do a first query (through Firebirdex.query) pending transactions start increasing again, just like before, so also through the pings. It looks like the allocate_statement in ping() is enough to increase pending transactions, once a begin_transaction has run.

As far as I understand the changes you have made, you have moved the begin_transaction from connect() to handle_prepare(), meaning that you are still doing a begin_transaction, but in this case upon the first prepare_statement. After that the transaction stays open, and pending transactions start increasing like before.

It is really the begin_transaction that seems to cause the problem.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

As you can see by trying, if begin_transaction() is not called in prepare_statement, an error will occur in prepare_statements().

Anyway, we need to start a transaction somewhere.
Isn't the problem to close(drop?) the transaction?

I would expect a commit or rollback to reduce the number of pending transactions, but is that not the case?

Should I do begin_transaction() first per connection and not do in handle_begin() ?
(In that case, I would need to call efirebird_protocol.commit_retainging() instead of efirebird_protocol.commit()).

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I was also starting to think that maybe it is not the database that does the auto_commit, but that it has to be implemented in the client by automatically doing a begin_transaction & commit when executing a single statement (when auto commit == true).

As I wrote a few posts up, when I put a hardcoded 0 in the trans_handle of the connect struct in efirebirdsql, a transaction is not started, and the connection can be used without an error. In that case you have to program transaction logic manually as a user of the Firebird library. That's totally fine, because that is exactly what you want if you set auto_commit to false,
I think we need this auto_commit = false feature, because otherwise it is not possible to work with actual transactions spanning multiple queries&updates.

If you would implement it in the client, I'm not sure wether it should be done in Firebirdex or efirebirdsql, because when it is implemented in Firebirdex the issue is solved only for users of Firebirdex, and sadly not for users of efirebirdsql.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Although I can't verify it, because I don't have the right documentation, maybe the meaning of trans_handle in the conn struct in efirebirdsql could be:

  • 0 -> this is a connection in auto_commit = false mode
  • 1 -> this is a connection in auto_commit = true mode (I noticed the trans_handle is automatically changed from :undefined to 1 when you do a begin_transaction with auto_commit = true).
  • :undefined -> for this connection it is not yet specified wether it will be used in auto_commit mode or not, but before executing statements through this connection you have to set it to 0 or 1.

If this is the case, a trans_handle with value 1 in the conn struct, can be used to detect wether after a statement was executed an automatic commit needs to be done.

What do you think?

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

I seem to remember reading somewhere that when the auto_commit connection option is set to true, there is a begin_transaction (with auto_commit = true) in the connect(), and whenever there is the execution of a statement it is followed by a commit and a new begin_transaction with again auto_commit = true.

Since it should normally be possible to start a normal transaction when your are in an auto_commit it would mean committing the current auto_commit transaction, and switching to auto_commit = false on the connection. When the programmer than commits or rolls back the normal transaction, you can again begin an auto_commit transaction when the auto_commit connection option = true, ...

There is some logic to think about, with maybe some kind of state machine, but it would allow for an implementation in efirebirdsql in stead of Firebirdex, so users of both libraries could benefit.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Because this auto_commit logic in the client is in my perception not so simple to implement and test, and because it could take some time I would like to suggest a short term & long term strategy:

Short term:

  1. keep auto_commit behaviour as it was before this issue was opened, and put a warning in the documentation, that for the time being, auto_commit behaviour is a bit different from normal auto_commit. When using auto_commit it should only be used for short lived connections (connect - execute (a few statements) - disconnect). The commit will happen, not immediately after every statement, but on disconnect.
    Technically, We don't do pings when auto_commit = true, since it will only be short lived connections, and there is not really a need for pings.
  2. For people who want to use long lived pooled connections (like myself :-D), auto_commit = true is not advised. Users must set the auto_commit connection parameter to false, and Users always must implement transaction logic (begin & commit/rollback).
    Technically, we put a 0 in the trans_handle of the connect struct when connecting, and we don't do a begin_transaction at all. It is the user who has to implement transaction logic in his code through Firebirdex.transaction(). We also don't do a commit when disconnecting because normally there is no transaction open.
    We also implement transaction logic in ping(), because we do want to do pings in long lived connections.

I think these changes are not so hard to implement in efirebirdsql in a reliable way.

Long term:

Implement auto_commit functionality in efirebirdsql with all the logic to automatically begin&commit transactions, and switching between auto_commit = true and false.

Do you think this is a good plan? Of course I'm willing to help implement and test this effort.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Sorry, I am not sure if what you are writing is valid.

On the other hand, I'm starting to think that calling begin_transaction() in handle_begin() is a bad idea.
I'll see if I can figure out a way to make begin_transaction() execute at most once per connection.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

My apologies. I'm indeed guessing and your are right in that my thinking could be completely invalid.
I very much appreciate all your efforts to solve this issue, and I will let you do the thinking because your are the creator of these great libraries.
Whenever you have a new possible solution I'm more than happy to do the tests for the increasing pending transactions.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

No problem.

I was not accusing you of anything.
I literally still don't understand it.
I will first try to work on what I do understand.

After that, your comments may be helpful.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

I changed a few things from #15 To #16
Does this solve the problem?

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

image

Same behaviour as in #15 : Pending transactions start to increase after executing the first Firebirdex.query()

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Same behaviour when I use a pool_size = 1, so I don't think it is linked to the number of connections.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

I can't think of a good solution.
I wonder if someone could look at the communication so far and send me a pull request to solve this problem.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Can you have a look at the following changes:

Based on efirebirdsql 0.9.0 I changed the following functions in firebirdsql_protocol:

-spec connect(string(), string(), string(), string(), list()) -> {ok, conn()} | {error, integer(), binary(), conn()}.
connect(Host, Username, Password, Database, Options) ->
    SockOptions = [{active, false}, {packet, raw}, binary],
    Port = proplists:get_value(port, Options, 3050),
    Charset = proplists:get_value(charset, Options, utf_8),
    IsCreateDB = proplists:get_value(createdb, Options, false),
    PageSize = proplists:get_value(pagesize, Options, 4096),
    AutoCommit = proplists:get_value(auto_commit, Options, true),
    Private = efirebirdsql_srp:get_private_key(),
    Public = efirebirdsql_srp:client_public(Private),
    case gen_tcp:connect(Host, Port, SockOptions) of
    {ok, Sock} ->
        Conn = #conn{
            sock=Sock,
            user=string:to_upper(Username),
            password=Password,
            charset=Charset,
            auto_commit=AutoCommit,
            client_private=Private,
            client_public=Public,
            auth_plugin=proplists:get_value(auth_plugin, Options, "Srp256"),
            wire_crypt=proplists:get_value(wire_crypt, Options, true),
            timezone=proplists:get_value(timezone, Options, nil)
        },
        case Conn#conn.auto_commit of
            true ->
                case connect_database(Conn, Host, Database, IsCreateDB, PageSize) of
                    {ok, C2} ->
                        case begin_transaction(AutoCommit, C2) of
                        {ok, C3} -> {ok, C3};
                        {error, ErrNo, Reason, C3} -> {error, ErrNo, Reason, C3}
                        end;
                    {error, ErrNo, Reason, C2} ->
                        {error, ErrNo, Reason, C2}
                end;
            false ->
                connect_database(Conn, Host, Database, IsCreateDB, PageSize)
        end;
    {error, Reason} ->
        {error, 0, atom_to_binary(Reason, latin1), #conn{
            user=string:to_upper(Username),
            password=Password,
            client_private=Private,
            client_public=Public,
            auth_plugin=proplists:get_value(auth_plugin, Options, "Srp256"),
            wire_crypt=proplists:get_value(wire_crypt, Options, true),
            auto_commit=proplists:get_value(auto_commit, Options, false),
            timezone=proplists:get_value(timezone, Options, nil)
        }}
    end.

-spec close(conn()) -> {ok, conn()} | {error, integer(), binary(), conn()}.
close(Conn) ->
    case Conn#conn.auto_commit of
        true ->  efirebirdsql_socket:send(Conn, efirebirdsql_op:op_commit_retaining(Conn#conn.trans_handle));
        false -> ok
    end,
    efirebirdsql_socket:send(Conn,
        efirebirdsql_op:op_detach(Conn#conn.db_handle)),
    case efirebirdsql_op:get_response(Conn) of
    {op_response, _, _} ->
        gen_tcp:close(Conn#conn.sock),
        {ok, Conn#conn{sock=undefined}};
    {error, ErrNo, Msg} ->
        {error, ErrNo, Msg, Conn}
    end.

-spec ping(conn()) -> ok | error.
ping(Conn) ->
    % Firebird 3.0+
%    efirebirdsql_socket:send(Conn,
%        efirebirdsql_op:op_ping()),
%    case efirebirdsql_op:get_response(Conn) of
%    {op_response, _, _} -> ok;
%    _ -> error
%    end.
    case Conn#conn.auto_commit of
    true -> ok;
    false ->
        case begin_transaction(false, Conn) of
        {ok, C2} -> 
            case allocate_statement(C2) of
            {ok, Stmt} -> 
                free_statement(C2, Stmt, drop),
                commit(C2),
                ok;
            {error, _errornr, msg} -> 
                rollback(C2),
                error
            end;
        _ -> error
        end
    end.

And in Firebirdex.Connection (based on fix_transaction #15) I changed the following functions:

  @impl true
  def connect(opts) do
    hostname = to_charlist(opts[:hostname])
    username = to_charlist(opts[:username])
    password = Keyword.get(opts, :password, System.get_env("FIREBIRD_PASSWORD"))
    password = to_charlist(password)
    database = to_charlist(opts[:database])
    case :efirebirdsql_protocol.connect(hostname, username, password, database, opts) do
      {:ok, conn} -> {:ok, %__MODULE__{conn: conn, transaction_status: :idle}}
      {:error, number, reason, _conn} ->
        {:error, %Error{number: number, reason: reason}}
    end
  end

  @impl true
  def ping( %__MODULE__{conn: conn} = state) do
    case :efirebirdsql_protocol.ping(conn) do
      :ok -> {:ok, state}
      :error -> {:disconnect,__MODULE__,state}
    end
  end

auto_commit = true

When the auto_commit option is true, basically the logic is more or less what it used to be before we started this whole issue.
There is again a begin_transaction with auto_commit in the connect() in efirebirdsql and ping() always just returns true.
This means the pending transactions only increase when executing statements. Ping's don't increase pending transactions anymore, because basically ping does nothing.
This is how Firebirdex and efirebirdsql used to work before we started working on this issue.

auto_commit = false

When I set auto_commit to false, I don't do a begin_transaction on connect(), and I now have to wrap all my database statements inside a manual transaction (Firebirdex.transaction) or I will get an error message when I try to execute a database statement, which is normal behaviour when auto_commit = false.
There are actual pings again by doing an allocate_statement and free_statement, but also here I have to wrap it inside a transaction. (I commented out the FB3 code)
When using auto_commit = false all my tests now show a flat line in the pending transactions graph, so no increasing pending transactions anymore.

For me this would be a good solution, since I work with long lived connections, and I don't mind wrapping my database statements inside transactions when I set auto_commit to false.

At the same time it allows you, if you want to, to still investigate further how to implement auto_commit logic when auto_commit = true, but for me the urgency is gone.

I'm very curious to see your feedback.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Hi, I have created pull requests for efirebirdsql and firebird to implement the behaviour above.
As far as I can see it does not brake anything when auto_commit = true, and requires users to use transactions when auto_commit = false.
It solves increasing pending transactions completely with auto_commit = false
I hope this is an acceptable solution for you.

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Thanks so much!

I have checked this pull request.
#17

At least it looks harmless, so I'll merge this pull request

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

Thanks. Don't forget it depends also on the pull request in efirebirdsql.

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

When I do a mix deps.get in my Elixir project it still gets efirebirdsql 0.8.5. Is that normal?

New:
  efirebirdsql 0.8.5
  firebirdex 0.3.4
* Getting firebirdex (Hex package)
* Getting codepagex (Hex package)
* Getting efirebirdsql (Hex package)

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Released just now

  • efirebridsql 0.9.1
  • firebirdex 0.3.5

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

If this has been resolved in firebirdex 0.3.5, please close this Issue

from firebirdex.

hermanius avatar hermanius commented on July 19, 2024

For my use case, where I use auto_commit = false, it is resolved.
I still have problems however when I try to install through hex.
mix.exs: {firebirdex, "~0.3")

New:
  efirebirdsql 0.8.5
  firebirdex 0.3.4

mix.exs: {firebirdex, "~0.3.5"}

** (Mix) No matching version for firebirdex ~> 0.3.5 (from: mix.exs) in registry

The latest version is: 0.3.4

mix.exs: {:firebirdex, git: "https://github.com/nakagami/firebirdex.git"}
This works:

Upgraded:
  efirebirdsql 0.8.5 => 0.9.1 (minor)

Is there some configuration you have to do maybe with hex.pm?

from firebirdex.

nakagami avatar nakagami commented on July 19, 2024

Sorry, README.md is wrong.
I have fixed the description in Github.

The description in hex.pm is still outdated, but I will update it in the next release.

from firebirdex.

Related Issues (9)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.