MXS-1629:make threadid unique

[ybbct]

I am contributing the new code of the whole pull request, including one or
several files that are either new files or modified ones, under the BSD-new
license.

[jhnwkmn]

Please change to session_id_lock

[jhnwkmn]

Please change to session_ids

[jhnwkmn]

Since this is now protected by a lock, there's no need to use atomic_add_uint32 but you can simply increment next_session_id.

[jhnwkmn]

I'm a bit concerned about putting the session id allocation behind a lock (during which a memory allocation may be made), because experience has thought that these locks start to hurt when the load gets high. I may want to do this so that each worker thread has a session id pool of its own (reserved from a global session id pool), from which it allocates session ids, so that session id allocation in most cases can be made without any locking being involved.

[svoj]

I'd argue this change as well. I'm not that much into MaxScale code, but the reasoning of changing 64bit counter to 32bit counter with mutex and id registry is totally unclear to me.

I'm not sure how often session_final_free() and session_get_next_id() are called, but with this patch they won't scale well anymore.

[ybbct]

@jhnwkmn

firstly, i will test this pr with following case, to measure how much impact with performance:

function event(thread_id)
   db_connect()
    rs = db_query("select 1;")
db_disconnect()

secondly, if this pr be accepted, we want go further to impl global threadid since currently client session id is different from server session id, although we have a mapping in mxs, but it not work in a cluster deploy;

our proposal is:

1. divide session id into two part,

    |31 - 24| 23 - 0|
   

   |-------|------|
   |nodeid | session id|

2. set this session id back to server

so if worker thread has a session id pool, another bits will be consumed

@svoj mysql protocol constrained threadid to be 32bits, in mysql_client.cc:272

// Send only the low 32bits in the handshake.
 gw_mysql_set_byte4(mysql_thread_id_num, (uint32_t)(protocol->thread_id));

[jhnwkmn]

@svoj The session id is visible to the client as the thread id, which is only 32-bit. So, even if the former is 64-bit it'll still wrap as far as the client is concerned if the situation is as in https://jira.mariadb.org/browse/MXS-1629
But come to think of it I don't know how MaxScale's current behaviour differs from that of the server itself. According to some documentation you should use SELECT CONNECTION_ID(); if the thread id becomes larger than 32-bit. So perhaps MaxScale should catch that and returns the actual 64-bit value.

[svoj]

Ok, that's a valid problem. Server uses 64bit CONNECTION_ID(), though I'm not sure if and how it appears in client-server protocol. Any reason not to make the whole 64bits visible to the client?

[jhnwkmn]

@svoj The problem is that in the handshake package you only have 32 bits.

[svoj]

I can imagine that the best solution would be either to extend handshake, or skip sending it to client, or skip using it in client for cases when uniqueness matters.

If none of the above works, I'd still keep it 64bits and use LF_HASH to track first 32bit uniqueness. This way scalability will be sacrificed less.

[vaintroub]

svoj , this is about MySQL client-server protocol. And we have the same problem with overflow in MariaDB server, which we ignored so far. I.e we can send different clients the same 32bit session ids Client typically use that for KILL CONNECTION, i.e when we go over 32bit, KILL might fail, or clients would KILL wrong connecton

[vaintroub]

@svoj : https://mariadb.com/kb/en/library/1-connecting-connecting/ , in the description of the packet, you can see int<4> connection id. Since the server cannot have more that 4 billions of connections at the same time, this connection id is not unreasonable. But changing protocol for that is not quite easy, since there are dozens of different native connectors for different languages. So connection ID should be 4 bytes, not 8, my opinion.

[jhnwkmn]

@vaintroub Do you suffer from an overflow problem in the server, or do you reuse connection ids (that have been closed)?

[vaintroub]

We suffer. MySQL does not suffer, since 5.7.5 https://mysqlserverteam.com/the-mysql-5-7-5-milestone-release-is-available/ (Proper Connection ID Handling, WL#7293)

[ybbct]

MySQL's solution is not efficient;

my_thread_id Global_THD_manager::get_new_thread_id()
{
  my_thread_id new_id;
  Mutex_lock lock(&LOCK_thread_ids);
  do {
    new_id= thread_id_counter++;
  } while (!thread_ids.insert_unique(new_id).second);
  return new_id;
}

std::pair<iterator, bool> insert_unique(const value_type &val)
  {
    std::pair<iterator, iterator> p= std::equal_range(begin(), end(), val);
    // p.first == p.second means we did not find it.
    if (p.first == p.second)
      return std::make_pair(insert(p.first, val), true);
    return std::make_pair(p.first, false);
  }

thread_ids is an array

[jhnwkmn]

I'll close this as we cannot have a global lock involved at each session id allocation. However, I've assigned MXS-1629 to myself and will provide a scalable solution.