MySQL Bugs: #103646: This judgment(trx->no < m_low_limit_no) in ReadView::prepare is not necessary

Bug #103646	This judgment(trx->no < m_low_limit_no) in ReadView::prepare is not necessary
Submitted:	11 May 2021 6:25	Modified:	24 May 2021 8:03
Reporter:	jie xu	Email Updates:
Status:	Verified	Impact on me:	None
Category:	MySQL Server: InnoDB Plugin storage engine	Severity:	S5 (Performance)
Version:	8.0	OS:	Any
Assigned to:		CPU Architecture:	Any

Description:
The judgment(trx->no < m_low_limit_no) in ReadView::prepare is not necessary.
The trx_no is assigned and the trx is added to serialisation_list hold trx_sys->mutex in trx_serialisation_number_get.
Holding trx_sys->mutex When get trx_no from serialisation_list and assign to m_low_limit_no in ReadView::prepare;
So trx->no can't bigger than m_low_limit_no in ReadView::prepare.

How to repeat:
no repeat.

Suggested fix:
remove this judgment(trx->no < m_low_limit_no) from ReadView::prepare

Hi Mr. xu,

Thank you for your bug report.

Can you please, describe what you are asking in more detail ????

Can you show what you are proposing in the code examples ????

Last, but not least, is the same code still present in 8.0.24 ??????

Thanks in advance.

========================================================
void ReadView::prepare(trx_id_t id) {
  ut_ad(mutex_own(&trx_sys->mutex));

  m_creator_trx_id = id;

  m_low_limit_no = m_low_limit_id = m_up_limit_id = trx_sys->max_trx_id;

  if (!trx_sys->rw_trx_ids.empty()) {
    copy_trx_ids(trx_sys->rw_trx_ids);
  } else {
    m_ids.clear();
  }

  ut_ad(m_up_limit_id <= m_low_limit_id);

  if (UT_LIST_GET_LEN(trx_sys->serialisation_list) > 0) {
    const trx_t *trx;

    trx = UT_LIST_GET_FIRST(trx_sys->serialisation_list);

    if (trx->no < m_low_limit_no) { // this line seem not necessary, m_low_limit_no is protected by trx_sys->mutex.
      m_low_limit_no = trx->no;
    }
  }

  ut_d(m_view_low_limit_no = m_low_limit_no);
  m_closed = false;
}
============================================================

============================================================
  trx_sys_mutex_enter();

  trx->no = trx_sys_get_new_trx_id();

  /* Update the latest transaction number. */
  ut_d(trx_sys->rw_max_trx_no = trx->no);

  /* Track the minimum serialisation number. */
  if (!trx->read_only) {
    UT_LIST_ADD_LAST(trx_sys->serialisation_list, trx); // when trx is added to serialisation_list, also protected by trx_sys->mutex.
    added_trx_no = true;
  } else {
    added_trx_no = false;
  }
============================================================

============================================================
static void trx_erase_lists(trx_t *trx, bool serialised, Gtid_desc &gtid_desc) {
  ut_ad(trx->id > 0);
  ut_ad(trx_sys_mutex_own());

  if (serialised) {
    /* Add GTID to be persisted to disk table. It must be done ...
    1.After the transaction is marked committed in undo. Otherwise
      GTID might get committed before the transaction commit on disk.
    2.Before it is removed from serialization list. Otherwise the transaction
      undo could get purged before persisting GTID on disk table. */
    if (gtid_desc.m_is_set) {
      auto &gtid_persistor = clone_sys->get_gtid_persistor();
      gtid_persistor.add(gtid_desc);
    }
    /* Do after adding GTID as trx_sys mutex could now be released and
    re-acquired while adding GTID and we still need to satisfy condition
    [2] above. */
    UT_LIST_REMOVE(trx_sys->serialisation_list, trx); // when remove trx from serialisation_list, is also protected by trx_sys->mutex.
  }
============================================================

Because of removing and adding a trx from serialisation_list are both protected by trx_sys->mutex, and the judgment(trx->no < m_low_limit_no) in ReadView::prepare is not necessary.

The code still present in 8.0.24

Hi Mr. xu,

Thank you for listing the functions in question. However, listing code without your relevant comments in it , does not make much sense. Please comment those lines of code that are relevant.

Thanks in advance.

Ok， Thanks for you reply.

The line 446 in read0read.cc is not necessary on 8.0.24.
m_low_limit_no is always bigger than the first trx no in trx_sys->serialisation_list

I remove this line and run it well in my environment.

Hello, Guys
I comment those lines of code that are relevant in above functions listing, Please check it.

Thank you.

Hi Mr. xu,

We agree with your analysis. Verified as reported.

Thank you!!