[jira] [Created] (IOTDB-229) Inconsistent usage of Marker in TsFile
Lei Rui created IOTDB-229: - Summary: Inconsistent usage of Marker in TsFile Key: IOTDB-229 URL: https://issues.apache.org/jira/browse/IOTDB-229 Project: Apache IoTDB Issue Type: Improvement Reporter: Lei Rui -- This message was sent by Atlassian Jira (v8.3.4#803005)
Re: [jira] [Created] (IOTDB-229) Inconsistent usage of Marker in TsFile
Hi, I do not know why the mail does not show the description... > As suggested by Jialin Qiao, an ideal case may be that ChunkMetaData.offsetOfChunkHeader refers to the position AFTER Marker_1 too. +1. > A solution is changing the name of the parameter from `markerRead` to `isMarkerExist` The correct meaning of the parameter should be `has the Marker been read`. If it is true, then the reader do not need to read 1 byte. I suggest to make the inconsistency to the above meaning. Best, --- Xiangdong Huang School of Software, Tsinghua University 黄向东 清华大学 软件学院 Lei Rui (Jira) 于2019年9月19日周四 下午1:27写道： > Lei Rui created IOTDB-229: > - > > Summary: Inconsistent usage of Marker in TsFile > Key: IOTDB-229 > URL: https://issues.apache.org/jira/browse/IOTDB-229 > Project: Apache IoTDB > Issue Type: Improvement > Reporter: Lei Rui > > > > > > > -- > This message was sent by Atlassian Jira > (v8.3.4#803005) >
Re: [jira] [Created] (IOTDB-229) Inconsistent usage of Marker in TsFile
Hi, > I do not know why the mail does not show the description... Sorry it is because I didn't know how to add the detail description when opening an issue on JIRA. Thankfully, I now know how to do that. Below is the raw description: ``` 1) Structure ChunkMetaData.offsetOfChunkHeader refers to the position of Marker_1, while TsDeviceMetadataIndex refers to the position AFTER Marker_2. This may impose unnecessary learning burdens on users. As suggested by Jialin Qiao, an ideal case may be that ChunkMetaData.offsetOfChunkHeader refers to the position AFTER Marker_1 too. 2) Annotation The parameter `boolean markerRead` is used across multiple methods. I found two annotations of it. One annotates "read marker (boolean type)" and the other annotates "Whether the marker of the CHUNK_HEADER has been read". When `markerRead` is FALSE, the former usage will add the current read offset by 1 while the latter usage will read the byte of the Marker. Although they both push forward the read process, the two different meanings of `markerRead` is a bit of a counter-intuitive. A solution is changing the name of the parameter from `markerRead` to `isMarkerExist`, and then when `isMarkerExist` is TRUE, the former usage can choose to add the current read offset by 1 to skip it while the latter usage can choose to read the byte of the Marker. Remind to maintain the tsfile/format-changelist.md if changes are made. ``` > If it is true, then the reader do not need to read 1 byte. Ok then just take care when using it. Lei Rui On 9/19/2019 14:12，Xiangdong Huang wrote： Hi, I do not know why the mail does not show the description... As suggested by Jialin Qiao, an ideal case may be that ChunkMetaData.offsetOfChunkHeader refers to the position AFTER Marker_1 too. +1. A solution is changing the name of the parameter from `markerRead` to `isMarkerExist` The correct meaning of the parameter should be `has the Marker been read`. If it is true, then the reader do not need to read 1 byte. I suggest to make the inconsistency to the above meaning. Best, --- Xiangdong Huang School of Software, Tsinghua University 黄向东 清华大学 软件学院 Lei Rui (Jira) 于2019年9月19日周四 下午1:27写道： Lei Rui created IOTDB-229: - Summary: Inconsistent usage of Marker in TsFile Key: IOTDB-229 URL: https://issues.apache.org/jira/browse/IOTDB-229 Project: Apache IoTDB Issue Type: Improvement Reporter: Lei Rui -- This message was sent by Atlassian Jira (v8.3.4#803005)
Re: [jira] [Created] (IOTDB-229) Inconsistent usage of Marker in TsFile
Hi, > Ok then just take care when using it. No, I think we need to fix the javadoc. Best, --- Xiangdong Huang School of Software, Tsinghua University 黄向东 清华大学 软件学院 Lei Rui 于2019年9月19日周四 下午2:31写道： > Hi, > > > > I do not know why the mail does not show the description... > Sorry it is because I didn't know how to add the detail description when > opening an issue on JIRA. Thankfully, I now know how to do that. > > > Below is the raw description: > ``` > 1) Structure > ChunkMetaData.offsetOfChunkHeader refers to the position of Marker_1, > while TsDeviceMetadataIndex refers to the position AFTER Marker_2. This may > impose unnecessary learning burdens on users. > As suggested by Jialin Qiao, an ideal case may be that > ChunkMetaData.offsetOfChunkHeader refers to the position AFTER Marker_1 too. > 2) Annotation > The parameter `boolean markerRead` is used across multiple methods. I > found two annotations of it. One annotates "read marker (boolean type)" and > the other annotates "Whether the marker of the CHUNK_HEADER has been read". > When `markerRead` is FALSE, the former usage will add the current read > offset by 1 while the latter usage will read the byte of the Marker. > Although they both push forward the read process, the two different > meanings of `markerRead` is a bit of a counter-intuitive. > A solution is changing the name of the parameter from `markerRead` to > `isMarkerExist`, and then when `isMarkerExist` is TRUE, the former usage > can choose to add the current read offset by 1 to skip it while the latter > usage can choose to read the byte of the Marker. > Remind to maintain the tsfile/format-changelist.md if changes are made. > ``` > > > > > > If it is true, then the reader do not need to read 1 byte. > Ok then just take care when using it. > > > Lei Rui > > > > > > > On 9/19/2019 14:12，Xiangdong Huang wrote： > Hi, > > I do not know why the mail does not show the description... > > As suggested by Jialin Qiao, an ideal case may be that > ChunkMetaData.offsetOfChunkHeader refers to the position AFTER Marker_1 > too. > > +1. > > A solution is changing the name of the parameter from `markerRead` to > `isMarkerExist` > > The correct meaning of the parameter should be `has the Marker been read`. > If it is true, then the reader do not need to read 1 byte. > > I suggest to make the inconsistency to the above meaning. > > Best, > --- > Xiangdong Huang > School of Software, Tsinghua University > > 黄向东 > 清华大学 软件学院 > > > Lei Rui (Jira) 于2019年9月19日周四 下午1:27写道： > > Lei Rui created IOTDB-229: > - > > Summary: Inconsistent usage of Marker in TsFile > Key: IOTDB-229 > URL: https://issues.apache.org/jira/browse/IOTDB-229 > Project: Apache IoTDB > Issue Type: Improvement > Reporter: Lei Rui > > > > > > > -- > This message was sent by Atlassian Jira > (v8.3.4#803005) > >
Re: [jira] [Created] (IOTDB-229) Inconsistent usage of Marker in TsFile
+1, generally the javadoc is not incorrect but can be written more clearly. Lei Rui On 9/19/2019 14:36，Xiangdong Huang wrote： Hi, Ok then just take care when using it. No, I think we need to fix the javadoc. Best, --- Xiangdong Huang School of Software, Tsinghua University 黄向东 清华大学 软件学院 Lei Rui 于2019年9月19日周四 下午2:31写道： Hi, I do not know why the mail does not show the description... Sorry it is because I didn't know how to add the detail description when opening an issue on JIRA. Thankfully, I now know how to do that. Below is the raw description: ``` 1) Structure ChunkMetaData.offsetOfChunkHeader refers to the position of Marker_1, while TsDeviceMetadataIndex refers to the position AFTER Marker_2. This may impose unnecessary learning burdens on users. As suggested by Jialin Qiao, an ideal case may be that ChunkMetaData.offsetOfChunkHeader refers to the position AFTER Marker_1 too. 2) Annotation The parameter `boolean markerRead` is used across multiple methods. I found two annotations of it. One annotates "read marker (boolean type)" and the other annotates "Whether the marker of the CHUNK_HEADER has been read". When `markerRead` is FALSE, the former usage will add the current read offset by 1 while the latter usage will read the byte of the Marker. Although they both push forward the read process, the two different meanings of `markerRead` is a bit of a counter-intuitive. A solution is changing the name of the parameter from `markerRead` to `isMarkerExist`, and then when `isMarkerExist` is TRUE, the former usage can choose to add the current read offset by 1 to skip it while the latter usage can choose to read the byte of the Marker. Remind to maintain the tsfile/format-changelist.md if changes are made. ``` If it is true, then the reader do not need to read 1 byte. Ok then just take care when using it. Lei Rui On 9/19/2019 14:12，Xiangdong Huang wrote： Hi, I do not know why the mail does not show the description... As suggested by Jialin Qiao, an ideal case may be that ChunkMetaData.offsetOfChunkHeader refers to the position AFTER Marker_1 too. +1. A solution is changing the name of the parameter from `markerRead` to `isMarkerExist` The correct meaning of the parameter should be `has the Marker been read`. If it is true, then the reader do not need to read 1 byte. I suggest to make the inconsistency to the above meaning. Best, --- Xiangdong Huang School of Software, Tsinghua University 黄向东 清华大学 软件学院 Lei Rui (Jira) 于2019年9月19日周四 下午1:27写道： Lei Rui created IOTDB-229: - Summary: Inconsistent usage of Marker in TsFile Key: IOTDB-229 URL: https://issues.apache.org/jira/browse/IOTDB-229 Project: Apache IoTDB Issue Type: Improvement Reporter: Lei Rui -- This message was sent by Atlassian Jira (v8.3.4#803005)
