| 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: | |
| Category: | MySQL Server: InnoDB storage engine | Severity: | S3 (Non-critical) |
| Version: | 8.0.* | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
[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.
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());