Bug #99596 create Fil_shard::create_node return wrong file node

Submitted: 15 May 2020 20:27 Modified: 20 May 2020 12:25
Reporter: Zongzhi Chen (OCA) Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S3 (Non-critical)
Version:8.0.* OS:Any
Assigned to: CPU Architecture:Any

[15 May 2020 20:27] Zongzhi Chen
Description:
In the file fil/fil0fil.cc
at the end of the function

fil_node_t *Fil_shard::create_node() {
   ...
  // Put the new fil_node_t to the end of space->files;
  // and space->files is a vector
  space->files.push_back(file);

  mutex_release();

  ut_a(space->id == TRX_SYS_SPACE ||
       space->id == dict_sys_t::s_log_space_first_id ||
       space->purpose == FIL_TYPE_TEMPORARY || space->files.size() == 1);
  
  // This function want to return the new created file_node_t
  // However, it will return the first fil_node_t, not the latest one in our function
  return (&space->files.front());
}

At most of the time, as the comment said the as most of time only one file will attach to a tablespace. However system tablespace and I thought include partitioned table can have multiple files. 

How to repeat:
read the code

Suggested fix:
return the latest of the fil_node_t

  return (&space->files.back());
[18 May 2020 12:31] MySQL Verification Team
Hi Mr. zongzhi,

Thank you for your bug report.

However, before we proceed, we do need some clarifications.

That particular method attaches a file to the tablespace and as each partition is a separate tablespace, why do you think that this is a problem.

Also, with millions of installations of partitioned InnoDB tables, don't you think that this would create a flurry of bug reports ?????
[19 May 2020 21:16] Zongzhi Chen
OK, I make a mistake about the tablespace id partition table..

However, how about the system tablespace. As the comment said 

  /** Files attached to this tablespace. Note: Only the system tablespace
  can have multiple files, this is a legacy issue. */
  Files files;

Why there won't be lots of bug report?

Cause most the the place use Fil_shard::create_node only need to check where the file_node is NULL of not. They won't care about return the fire item of the files vector or the last item of the vector, such as:

```
 5309   fil_node_t *file_node =
 5310       shard->create_node(path, size, space, false, punch_hole, atomic_write);
 5311
 5312   err = (file_node == nullptr) ? DB_ERROR : DB_SUCCESS;
```

```
 5509   const fil_node_t *file =
 5510       shard->create_node(df.filepath(), 0, space, false, atomic_write, false);
 5511
 5512   if (file == nullptr) {
 5513     return (DB_ERROR);
 5514   }
```
```
5855   const fil_node_t *file;
 5856
 5857   file = create_node(df.filepath(), 0, space, false, true, false);
 5858
 5859   ut_a(file != nullptr);
```

There is only one place that use the file->name

```
 2284 char *fil_node_create(const char *name, page_no_t size, fil_space_t *space,
 2285                       bool is_raw, bool atomic_write, page_no_t max_pages) {
 2286   auto shard = fil_system->shard_by_id(space->id);
 2290   file = shard->create_node(name, size, space, is_raw,
 2291                             IORequest::is_punch_hole_supported(), atomic_write,
 2292                             max_pages);
 2293
 2294   return (file == nullptr ? nullptr : file->name);
 2295 }
```

However, the upper layer call fil_node_create won't care about the file->name result

```
 4399   char *fn =
 4400       fil_node_create(file_name.c_str(), n_pages, space, false, atomic_write);
 4401   if (fn == nullptr) {
 4402     ib::error(ER_IB_MSG_1082, space_name.c_str());
 4403     return (false);
 4404   }
```

so this is still a mistake to return the first item of the vector..
[20 May 2020 12:25] MySQL Verification Team
Hi Mr. zongzhi.

Regarding system tablespace, which can have multiple files, you seem to be correct.

Verified as reported.

Thank you for your contribution.

Portions of this website are copyright © 2001, 2002 The PHP Group

Page generated in 0.051 sec. using MySQL 8.4.7-u6-cloud

Timestamp references displayed by the system are UTC.
Content reproduced on this site is the property of the respective copyright holders.
It is not reviewed in advance by Oracle and does not necessarily represent the opinion of Oracle or any other party.