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: | |
| 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]
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.
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());