Bug #99596 create Fil_shard::create_node return wrong file node
Submitted: 15 May 2020 20:27 Modified: 20 May 2020 12:25
Reporter: chen zongzhi (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] chen zongzhi
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] chen zongzhi
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.