You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/07/14 00:20:51 UTC
[GitHub] [trafficserver] bneradt opened a new pull request #8062: Log meta file: ensure the entire contents are written
bneradt opened a new pull request #8062:
URL: https://github.com/apache/trafficserver/pull/8062
Updating the write of the meta file content for log rotation to ensure
that the entire line is written. If not, then the file will be
ill-formed which will lead eventually to a crash.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #8062: Log meta file: ensure the entire contents are written
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #8062:
URL: https://github.com/apache/trafficserver/pull/8062#discussion_r670623921
##########
File path: src/tscore/ink_sock.cc
##########
@@ -111,6 +112,26 @@ safe_clr_fl(int fd, int arg)
return flags;
}
+int
+safe_write(int fd, const char *buffer, int len)
+{
+ int num_written = 0;
+ while (num_written < len) {
Review comment:
I think this should be re-ordered such that the `write` is attempted first, and only if that doesn't fully succeed should `write_ready` be invoked. That reduces the impact of calling this for the (by far) common case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a change in pull request #8062: Log meta file: ensure the entire contents are written
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #8062:
URL: https://github.com/apache/trafficserver/pull/8062#discussion_r669926103
##########
File path: include/tscore/BaseLogFile.h
##########
@@ -99,6 +99,18 @@ class BaseMetaInfo
void _read_from_file();
void _write_to_file();
+
+ /** Repeat write calls to fd until all len bytes are written.
+ *
+ * @param[in] fd The file descriptor to write to.
+ * @param[in] buffer The buffer of bytes to write into fd.
+ * @param[in] len The number of bytes in buffer to write into fd.
+ *
+ * @return The number of bytes written or -1 if there was an error writing to
+ * the file descriptor. If -1 is returned, errno should indicate why the
+ * write failed.
+ */
+ int _write_till_buffer_is_drained(int fd, char const *buffer, int len);
Review comment:
Alan suggested offline adding a `safe_write` method. That's a good idea, so I'll look into that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] SolidWallOfCode commented on pull request #8062: Log meta file: ensure the entire contents are written
Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on pull request #8062:
URL: https://github.com/apache/trafficserver/pull/8062#issuecomment-880868296
I think it's good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt merged pull request #8062: Log meta file: ensure the entire contents are written
Posted by GitBox <gi...@apache.org>.
bneradt merged pull request #8062:
URL: https://github.com/apache/trafficserver/pull/8062
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a change in pull request #8062: Log meta file: ensure the entire contents are written
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #8062:
URL: https://github.com/apache/trafficserver/pull/8062#discussion_r670643846
##########
File path: src/tscore/ink_sock.cc
##########
@@ -111,6 +112,26 @@ safe_clr_fl(int fd, int arg)
return flags;
}
+int
+safe_write(int fd, const char *buffer, int len)
+{
+ int num_written = 0;
+ while (num_written < len) {
Review comment:
Good point. Thanks Alan.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org