mongodb-erlang icon indicating copy to clipboard operation
mongodb-erlang copied to clipboard

mc_worker_logic:make_request was being called with the wrong DB

Open stephb9959 opened this issue 4 years ago • 4 comments

Although the code was properly figuring out the right DB, it would always pass the Connection DB to the make_request.

stephb9959 avatar Sep 02 '19 06:09 stephb9959

Hi, Can you please fix the tests?

comtihon avatar Sep 02 '19 18:09 comtihon

Hi Val!

So I tried to add a dumb test:

ok = mongo_api:ensure_index(Pid, Collection, #{<<"key">> => {<<"cid">>, 1, <<"ts">>, 1}} , <<"test2">>), ok = mongo_api:ensure_index(Pid, Collection, {<<"key">>, {<<"z_first">>, 1, <<"a_last">>, 1}}),

That makes the tests fail. I think the problem is in the protocol. Here is what I am finding, and please correct me if I am wrong.

  • If you connect to a specific DB, you cannot switch DBs afterward on the same connection.
  • If you do not connect with a DB, you may specify a DB on any subsequent call, and you may change DBs.

So in order to make this test, I would need to add a second connection and write a test using that connection, and not setting the database in that connection.

I came up with this fix after trying to create indexes for about 6 hours and no being able to. After looking at more traces than I care to, and massive printf() debugging, I followed the code into mc_worker.erl. Where ar line 76 it is not using the database name that was obtained earlier. So in the case where you do not connect with a DB, that line would always try to use "undefined". The fix was to use the calculated value of DB, which turns out to be right.

Let me know if you still want me to create the test. It would be a large undertaking.

Q: The operation to create an index does not seem to return an error, ever. I followed the code and it never looks for a return value from Mongo. Am I mistaken?

Cheers!

On Mon, Sep 2, 2019 at 11:40 AM Val [email protected] wrote:

Hi, Can you please fix the tests?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/comtihon/mongodb-erlang/pull/217?email_source=notifications&email_token=AAOF7RCPZFSV2OS3VXI4H7TQHVM25A5CNFSM4ISZ3IIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5WMGSI#issuecomment-527221577, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOF7RANI3MWDQ3CAOVLWGLQHVM25ANCNFSM4ISZ3IIA .

-- Stéphane Bourque

stephb9959 avatar Sep 03 '19 03:09 stephb9959

Hi, mc_worker checks only ok when sending ensure index request.

{ok, _, _} =
    mc_worker_logic:make_request(
      Socket,
      NetModule,
      ConnState#conn_state.database,
      #insert{collection = mc_worker_logic:update_dbcoll(Coll, <<"system.indexes">>), documents = [Index]}),

mongo_api:ensure_index doesn't send anything to the database. It just prepares the request. You can rename it in this pr, if you find it confusing.

As far as I remember you always pass authentication against the specific database, so you can't just switch database in the same connection. I am not talking about creating a new test, but all the old tests are failing for some reason. Although I am not sure it is because of your changes, as the latest master is also red.

comtihon avatar Sep 03 '19 07:09 comtihon

I think my application for Mongo is a little different. The DB is in a private cloud behind firewalls and applications marshaling all access. So I have no security on Mongo at all. That might be why this is working for me.

On Tue, Sep 3, 2019 at 12:28 AM Val [email protected] wrote:

Hi, mc_worker checks only ok when sending ensure index request.

{ok, _, _} = mc_worker_logic:make_request( Socket, NetModule, ConnState#conn_state.database, #insert{collection = mc_worker_logic:update_dbcoll(Coll, <<"system.indexes">>), documents = [Index]}),

mongo_api:ensure_index doesn't send anything to the database. It just prepares the request. You can rename it in this pr, if you find it confusing.

As far as I remember you always pass authentication against the specific database, so you can't just switch database in the same connection. I am not talking about creating a new test, but all the old tests are failing for some reason. Although I am not sure it is because of your changes, as the latest master is also red.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/comtihon/mongodb-erlang/pull/217?email_source=notifications&email_token=AAOF7RGUQCACL6P25HQCH5LQHYGZXA5CNFSM4ISZ3IIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5XJBLY#issuecomment-527339695, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOF7RGGZ6F3HPW2RBGKTELQHYGZXANCNFSM4ISZ3IIA .

-- Stéphane Bourque

stephb9959 avatar Sep 03 '19 14:09 stephb9959