You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/06/01 10:55:47 UTC

[GitHub] [incubator-doris] spaces-X opened a new pull request #3741: Ignore broken disks when BE starts up

spaces-X opened a new pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741


   Ignore **broken disks** from storage_root_path:
   
   - add read and write check for each dir in storage_root_path and ignore broken disks.
   - add AutoDeallocator.h to manage the lifecycle of allocated data by malloc or posix_memalign.
   - rewrite DataDir::_read_and_write_test_file function.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] kangkaisen commented on a change in pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#discussion_r433296327



##########
File path: be/src/util/auto_deallocator.h
##########
@@ -0,0 +1,76 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+# pragma once
+#include <stdlib.h>
+
+namespace doris{
+
+// A move-only type which manage the lifecycle of allocated data by malloc or posix_memalign
+// use free() to de-allocate data
+// Usage example:
+//  {
+//      ...
+//      char *s = nullptr;
+//      posix_memalign((void **)s, 512, 4096);
+//      AutoDeallocator tmps(s);
+//      char *p = (char *)malloc(100*sizeof(char));
+//      AutoDeallocator tmpp(p);
+//      ...
+//  }
+    class AutoDeallocator {

Review comment:
       We don't need this class. std::unique_ptr and std::shared_ptr is enough




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] spaces-X commented on pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
spaces-X commented on pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#issuecomment-636865723


   > Hi @spaces-X, Could you create an issue to describe what problem you try to solve?
   
   Done [issue](https://github.com/apache/incubator-doris/issues/3746) 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#issuecomment-638215306


   Compilation failed. Did you miss `util/auto_deallocator.h`?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#issuecomment-638796007


   Hi @chaoyli,  Any more comment?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#discussion_r434385961



##########
File path: be/src/olap/utils.cpp
##########
@@ -973,6 +979,104 @@ OLAPStatus copy_file(const string& src, const string& dest) {
     
     return res;
 }
+OLAPStatus read_write_test_file(const string& test_file_path) {
+    if (access(test_file_path.c_str(), F_OK) == 0) {
+        if (remove(test_file_path.c_str()) != 0) {
+            char errmsg[64];
+            LOG(WARNING) << "fail to delete test file. "
+                         << "path=" << test_file_path
+                         << ", errno=" << errno << ", err=" << strerror_r(errno, errmsg, 64);
+            return OLAP_ERR_IO_ERROR;
+        }
+    } else {
+        if (errno != ENOENT) {
+            char errmsg[64];
+            LOG(WARNING) << "fail to access test file. "
+                         << "path=" << test_file_path
+                         << ", errno=" << errno << ", err=" << strerror_r(errno, errmsg, 64);
+            return OLAP_ERR_IO_ERROR;
+        }
+    }
+    OLAPStatus res = OLAP_SUCCESS;
+    FileHandler file_handler;
+    if ((res = file_handler.open_with_mode(test_file_path.c_str(),
+                                           O_RDWR | O_CREAT | O_DIRECT,
+                                           S_IRUSR | S_IWUSR)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to create test file. path=" << test_file_path;
+        return res;
+    }
+    const size_t TEST_FILE_BUF_SIZE = 4096;
+    const size_t DIRECT_IO_ALIGNMENT = 512;
+    char *write_test_buff = nullptr;
+    char *read_test_buff = nullptr;
+    if (posix_memalign((void**) &write_test_buff, DIRECT_IO_ALIGNMENT, TEST_FILE_BUF_SIZE)!= 0) {
+        LOG(WARNING) << "fail to allocate write buffer memory. size=" <<  TEST_FILE_BUF_SIZE;
+        return OLAP_ERR_MALLOC_ERROR;
+    }
+    unique_ptr<char, decltype(&std::free)> write_buff (write_test_buff, &std::free);
+    if (posix_memalign((void**) &read_test_buff, DIRECT_IO_ALIGNMENT, TEST_FILE_BUF_SIZE)!= 0) {
+        LOG(WARNING) << "fail to allocate read buffer memory. size=" <<  TEST_FILE_BUF_SIZE;
+        return OLAP_ERR_MALLOC_ERROR;
+    }
+    unique_ptr<char, decltype(&std::free)> read_buff (read_test_buff, &std::free);
+    // generate random numbers
+    uint32_t rand_seed = static_cast<uint32_t>(time(NULL));
+    for (size_t i = 0; i < TEST_FILE_BUF_SIZE; ++i) {
+        int32_t tmp_value = rand_r(&rand_seed);
+        write_test_buff[i] = static_cast<char>(tmp_value);
+    }
+    if ((res = file_handler.pwrite(write_buff.get(), TEST_FILE_BUF_SIZE, SEEK_SET)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to write test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if ((res = file_handler.pread(read_buff.get(), TEST_FILE_BUF_SIZE, SEEK_SET)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to read test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if (memcmp(write_buff.get(), read_buff.get(), TEST_FILE_BUF_SIZE) != 0) {
+        LOG(WARNING) << "the test file write_buf and read_buf not equal, [file_name = " << test_file_path << "]";
+        return OLAP_ERR_TEST_FILE_ERROR;
+    }
+    if ((res = file_handler.close()) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to close test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if (remove(test_file_path.c_str()) != 0) {
+        char errmsg[64];
+        VLOG(3) << "fail to delete test file. [err='" << strerror_r(errno, errmsg, 64)
+                << "' path='" << test_file_path << "']";
+        return OLAP_ERR_IO_ERROR;
+    }
+    return res;
+}
+
+bool check_datapath_rw(const string& path) {
+    if (!check_dir_existed(path))
+        return false;
+    string file_path = path + "/.read_write_test_file";
+    try {
+        OLAPStatus res = read_write_test_file(file_path);
+        return res == OLAP_SUCCESS;
+    } catch (...) {
+        // do nothing
+    }
+    LOG(WARNING) << "error when try to read and write temp file under the data path and return false. [path=" << path << "]";
+    return false;
+}
+
+bool check_dir_existed(const string& path) {
+    boost::filesystem::path p(path.c_str());
+
+    try {
+        return boost::filesystem::exists(p);

Review comment:
       You can use check_exist function in util/file_utils.h.
   Boots::filesystem::exist will be replace by util/file_utils.h.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#issuecomment-636845240


   Hi @spaces-X, Could you create an issue to describe what problem you try to solve?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#issuecomment-637919801


   > Could I merge this pr to master?
   > Or what should I do next?
   
   Hi, you don't need to do anything. This pr will be merged after being approved for a period of time,  
   about a day, in case some other reviews have review comments.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] kangkaisen commented on a change in pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#discussion_r433782750



##########
File path: be/src/service/doris_main.cpp
##########
@@ -136,6 +136,25 @@ int main(int argc, char** argv) {
         LOG(FATAL) << "parse config storage path failed, path=" << doris::config::storage_root_path;
         exit(-1);
     }
+    auto it = paths.begin();

Review comment:
       OK, I see. Thanks.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] spaces-X commented on pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
spaces-X commented on pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#issuecomment-637970984


   > > Could I merge this pr to master?
   > > Or what should I do next?
   > 
   > Hi, you don't need to do anything. This pr will be merged after being approved for a period of time,
   > about a day, in case some other reviews have review comments.
   
   Pull Request for my first time. 
   Thanks a lot.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] kangkaisen commented on a change in pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#discussion_r433291742



##########
File path: be/src/service/doris_main.cpp
##########
@@ -136,6 +136,25 @@ int main(int argc, char** argv) {
         LOG(FATAL) << "parse config storage path failed, path=" << doris::config::storage_root_path;
         exit(-1);
     }
+    auto it = paths.begin();

Review comment:
       ignore_broken_disk has handled in `parse_conf_store_paths`.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] spaces-X commented on a change in pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
spaces-X commented on a change in pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#discussion_r433355972



##########
File path: be/src/service/doris_main.cpp
##########
@@ -136,6 +136,25 @@ int main(int argc, char** argv) {
         LOG(FATAL) << "parse config storage path failed, path=" << doris::config::storage_root_path;
         exit(-1);
     }
+    auto it = paths.begin();

Review comment:
       Yeah, but i think it just checked whether the data path is existed. It didn't attempt to read or write test files in data path.
   The current version didn't cover the following situation:
   - Data path is existed but the disk is broken.
   - We can use command 'cd path' into the data path directory, but we can not create or write files for lots of reasons, for instance, no write permission.
   
   
   So i try to add the read_write_check step before BE starts up.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on a change in pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#discussion_r434387237



##########
File path: be/src/olap/utils.cpp
##########
@@ -973,6 +979,104 @@ OLAPStatus copy_file(const string& src, const string& dest) {
     
     return res;
 }
+OLAPStatus read_write_test_file(const string& test_file_path) {
+    if (access(test_file_path.c_str(), F_OK) == 0) {
+        if (remove(test_file_path.c_str()) != 0) {
+            char errmsg[64];
+            LOG(WARNING) << "fail to delete test file. "
+                         << "path=" << test_file_path
+                         << ", errno=" << errno << ", err=" << strerror_r(errno, errmsg, 64);
+            return OLAP_ERR_IO_ERROR;
+        }
+    } else {
+        if (errno != ENOENT) {
+            char errmsg[64];
+            LOG(WARNING) << "fail to access test file. "
+                         << "path=" << test_file_path
+                         << ", errno=" << errno << ", err=" << strerror_r(errno, errmsg, 64);
+            return OLAP_ERR_IO_ERROR;
+        }
+    }
+    OLAPStatus res = OLAP_SUCCESS;
+    FileHandler file_handler;
+    if ((res = file_handler.open_with_mode(test_file_path.c_str(),
+                                           O_RDWR | O_CREAT | O_DIRECT,
+                                           S_IRUSR | S_IWUSR)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to create test file. path=" << test_file_path;
+        return res;
+    }
+    const size_t TEST_FILE_BUF_SIZE = 4096;
+    const size_t DIRECT_IO_ALIGNMENT = 512;
+    char *write_test_buff = nullptr;
+    char *read_test_buff = nullptr;
+    if (posix_memalign((void**) &write_test_buff, DIRECT_IO_ALIGNMENT, TEST_FILE_BUF_SIZE)!= 0) {
+        LOG(WARNING) << "fail to allocate write buffer memory. size=" <<  TEST_FILE_BUF_SIZE;
+        return OLAP_ERR_MALLOC_ERROR;
+    }
+    unique_ptr<char, decltype(&std::free)> write_buff (write_test_buff, &std::free);
+    if (posix_memalign((void**) &read_test_buff, DIRECT_IO_ALIGNMENT, TEST_FILE_BUF_SIZE)!= 0) {
+        LOG(WARNING) << "fail to allocate read buffer memory. size=" <<  TEST_FILE_BUF_SIZE;
+        return OLAP_ERR_MALLOC_ERROR;
+    }
+    unique_ptr<char, decltype(&std::free)> read_buff (read_test_buff, &std::free);
+    // generate random numbers
+    uint32_t rand_seed = static_cast<uint32_t>(time(NULL));
+    for (size_t i = 0; i < TEST_FILE_BUF_SIZE; ++i) {
+        int32_t tmp_value = rand_r(&rand_seed);
+        write_test_buff[i] = static_cast<char>(tmp_value);
+    }
+    if ((res = file_handler.pwrite(write_buff.get(), TEST_FILE_BUF_SIZE, SEEK_SET)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to write test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if ((res = file_handler.pread(read_buff.get(), TEST_FILE_BUF_SIZE, SEEK_SET)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to read test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if (memcmp(write_buff.get(), read_buff.get(), TEST_FILE_BUF_SIZE) != 0) {
+        LOG(WARNING) << "the test file write_buf and read_buf not equal, [file_name = " << test_file_path << "]";
+        return OLAP_ERR_TEST_FILE_ERROR;
+    }
+    if ((res = file_handler.close()) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to close test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if (remove(test_file_path.c_str()) != 0) {
+        char errmsg[64];
+        VLOG(3) << "fail to delete test file. [err='" << strerror_r(errno, errmsg, 64)
+                << "' path='" << test_file_path << "']";
+        return OLAP_ERR_IO_ERROR;
+    }
+    return res;
+}
+
+bool check_datapath_rw(const string& path) {
+    if (!check_dir_existed(path))
+        return false;
+    string file_path = path + "/.read_write_test_file";
+    try {
+        OLAPStatus res = read_write_test_file(file_path);
+        return res == OLAP_SUCCESS;
+    } catch (...) {
+        // do nothing
+    }
+    LOG(WARNING) << "error when try to read and write temp file under the data path and return false. [path=" << path << "]";
+    return false;
+}
+
+bool check_dir_existed(const string& path) {
+    boost::filesystem::path p(path.c_str());
+
+    try {
+        return boost::filesystem::exists(p);

Review comment:
       This is related to this issue https://github.com/apache/incubator-doris/issues/2857




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] spaces-X commented on pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
spaces-X commented on pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#issuecomment-637918458


   Could I merge this pr to master?
   Or what should I do next?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] spaces-X commented on pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
spaces-X commented on pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#issuecomment-638667370


   > Compilation failed. Did you miss `util/auto_deallocator.h`?
   
   Hi, i have removed the redundant `include` line, and rebuild it successfully in my machine. 
   How to handle with the remaining requested changes created by reviewer 'chaoyli', which have already been changed.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] spaces-X commented on pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
spaces-X commented on pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#issuecomment-638253988


   > auto_deallocator
   
   sorry, i forgot to remove that line


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#discussion_r433252027



##########
File path: be/src/olap/utils.cpp
##########
@@ -973,6 +979,108 @@ OLAPStatus copy_file(const string& src, const string& dest) {
     
     return res;
 }
+OLAPStatus read_write_test_file(const string& test_file_path) {
+    if (access(test_file_path.c_str(), F_OK) == 0) {
+        if (remove(test_file_path.c_str()) != 0) {
+            char errmsg[64];
+            LOG(WARNING) << "fail to delete test file. "
+                         << "path=" << test_file_path
+                         << ", errno=" << errno << ", err=" << strerror_r(errno, errmsg, 64);
+            return OLAP_ERR_IO_ERROR;
+        }
+    } else {
+        if (errno != ENOENT) {
+            char errmsg[64];
+            LOG(WARNING) << "fail to access test file. "
+                         << "path=" << test_file_path
+                         << ", errno=" << errno << ", err=" << strerror_r(errno, errmsg, 64);
+            return OLAP_ERR_IO_ERROR;
+        }
+    }
+    OLAPStatus res = OLAP_SUCCESS;
+    FileHandler file_handler;
+    if ((res = file_handler.open_with_mode(test_file_path.c_str(),
+                                           O_RDWR | O_CREAT | O_DIRECT,
+                                           S_IRUSR | S_IWUSR)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to create test file. path=" << test_file_path;
+        return res;
+    }
+    const size_t TEST_FILE_BUF_SIZE = 4096;
+    const size_t DIRECT_IO_ALIGNMENT = 512;
+    char *write_test_buff = nullptr;
+    char *read_test_buff = nullptr;
+    if (posix_memalign((void**) &write_test_buff, DIRECT_IO_ALIGNMENT, TEST_FILE_BUF_SIZE)!= 0) {
+        LOG(WARNING) << "fail to allocate write buffer memory. size=" <<  TEST_FILE_BUF_SIZE;
+        return OLAP_ERR_MALLOC_ERROR;
+    }
+    AutoDeallocator write_buff(write_test_buff);
+    if (posix_memalign((void**) &read_test_buff, DIRECT_IO_ALIGNMENT, TEST_FILE_BUF_SIZE)!= 0) {
+        LOG(WARNING) << "fail to allocate read buffer memory. size=" <<  TEST_FILE_BUF_SIZE;
+        return OLAP_ERR_MALLOC_ERROR;
+    }
+    AutoDeallocator read_buff(read_test_buff);
+    // generate random numbers
+    uint32_t rand_seed = static_cast<uint32_t>(time(NULL));
+    for (size_t i = 0; i < TEST_FILE_BUF_SIZE; ++i) {
+        int32_t tmp_value = rand_r(&rand_seed);
+        write_test_buff[i] = static_cast<char>(tmp_value);
+    }
+    if ((res = file_handler.pwrite(write_buff.get(), TEST_FILE_BUF_SIZE, SEEK_SET)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to write test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if ((res = file_handler.pread(read_buff.get(), TEST_FILE_BUF_SIZE, SEEK_SET)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to read test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if (memcmp(write_buff.get(), read_buff.get(), TEST_FILE_BUF_SIZE) != 0) {
+        LOG(WARNING) << "the test file write_buf and read_buf not equal, [file_name = " << test_file_path << "]";
+        return OLAP_ERR_TEST_FILE_ERROR;
+    }
+    if ((res = file_handler.close()) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to close test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if (remove(test_file_path.c_str()) != 0) {
+        char errmsg[64];
+        VLOG(3) << "fail to delete test file. [err='" << strerror_r(errno, errmsg, 64)
+                << "' path='" << test_file_path << "']";
+        return OLAP_ERR_IO_ERROR;
+    }
+    return res;
+}
+
+bool check_datapath_rw(const string& path) {
+    if (!check_dir_existed(path))
+        return false;
+    string file_path = path + "/.read_write_test_file";
+    try {
+        OLAPStatus res = read_write_test_file(file_path);
+        return res == OLAP_SUCCESS;
+    } catch (...) {
+        // do nothing
+    }
+    LOG(WARNING) << "error when try to read and write temp file under the data path and return false. [path=" << path << "]";
+    return false;
+}
+
+bool check_dir_existed(const string& path) {
+    boost::filesystem::path p(path.c_str());
+
+    try {
+        if (boost::filesystem::exists(p)) {

Review comment:
       Just use:
   `return boost::filesystem::exists(p);`




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] spaces-X commented on a change in pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
spaces-X commented on a change in pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741#discussion_r434699516



##########
File path: be/src/olap/utils.cpp
##########
@@ -973,6 +979,104 @@ OLAPStatus copy_file(const string& src, const string& dest) {
     
     return res;
 }
+OLAPStatus read_write_test_file(const string& test_file_path) {
+    if (access(test_file_path.c_str(), F_OK) == 0) {
+        if (remove(test_file_path.c_str()) != 0) {
+            char errmsg[64];
+            LOG(WARNING) << "fail to delete test file. "
+                         << "path=" << test_file_path
+                         << ", errno=" << errno << ", err=" << strerror_r(errno, errmsg, 64);
+            return OLAP_ERR_IO_ERROR;
+        }
+    } else {
+        if (errno != ENOENT) {
+            char errmsg[64];
+            LOG(WARNING) << "fail to access test file. "
+                         << "path=" << test_file_path
+                         << ", errno=" << errno << ", err=" << strerror_r(errno, errmsg, 64);
+            return OLAP_ERR_IO_ERROR;
+        }
+    }
+    OLAPStatus res = OLAP_SUCCESS;
+    FileHandler file_handler;
+    if ((res = file_handler.open_with_mode(test_file_path.c_str(),
+                                           O_RDWR | O_CREAT | O_DIRECT,
+                                           S_IRUSR | S_IWUSR)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to create test file. path=" << test_file_path;
+        return res;
+    }
+    const size_t TEST_FILE_BUF_SIZE = 4096;
+    const size_t DIRECT_IO_ALIGNMENT = 512;
+    char *write_test_buff = nullptr;
+    char *read_test_buff = nullptr;
+    if (posix_memalign((void**) &write_test_buff, DIRECT_IO_ALIGNMENT, TEST_FILE_BUF_SIZE)!= 0) {
+        LOG(WARNING) << "fail to allocate write buffer memory. size=" <<  TEST_FILE_BUF_SIZE;
+        return OLAP_ERR_MALLOC_ERROR;
+    }
+    unique_ptr<char, decltype(&std::free)> write_buff (write_test_buff, &std::free);
+    if (posix_memalign((void**) &read_test_buff, DIRECT_IO_ALIGNMENT, TEST_FILE_BUF_SIZE)!= 0) {
+        LOG(WARNING) << "fail to allocate read buffer memory. size=" <<  TEST_FILE_BUF_SIZE;
+        return OLAP_ERR_MALLOC_ERROR;
+    }
+    unique_ptr<char, decltype(&std::free)> read_buff (read_test_buff, &std::free);
+    // generate random numbers
+    uint32_t rand_seed = static_cast<uint32_t>(time(NULL));
+    for (size_t i = 0; i < TEST_FILE_BUF_SIZE; ++i) {
+        int32_t tmp_value = rand_r(&rand_seed);
+        write_test_buff[i] = static_cast<char>(tmp_value);
+    }
+    if ((res = file_handler.pwrite(write_buff.get(), TEST_FILE_BUF_SIZE, SEEK_SET)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to write test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if ((res = file_handler.pread(read_buff.get(), TEST_FILE_BUF_SIZE, SEEK_SET)) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to read test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if (memcmp(write_buff.get(), read_buff.get(), TEST_FILE_BUF_SIZE) != 0) {
+        LOG(WARNING) << "the test file write_buf and read_buf not equal, [file_name = " << test_file_path << "]";
+        return OLAP_ERR_TEST_FILE_ERROR;
+    }
+    if ((res = file_handler.close()) != OLAP_SUCCESS) {
+        LOG(WARNING) << "fail to close test file. [file_name=" << test_file_path << "]";
+        return res;
+    }
+    if (remove(test_file_path.c_str()) != 0) {
+        char errmsg[64];
+        VLOG(3) << "fail to delete test file. [err='" << strerror_r(errno, errmsg, 64)
+                << "' path='" << test_file_path << "']";
+        return OLAP_ERR_IO_ERROR;
+    }
+    return res;
+}
+
+bool check_datapath_rw(const string& path) {
+    if (!check_dir_existed(path))
+        return false;
+    string file_path = path + "/.read_write_test_file";
+    try {
+        OLAPStatus res = read_write_test_file(file_path);
+        return res == OLAP_SUCCESS;
+    } catch (...) {
+        // do nothing
+    }
+    LOG(WARNING) << "error when try to read and write temp file under the data path and return false. [path=" << path << "]";
+    return false;
+}
+
+bool check_dir_existed(const string& path) {
+    boost::filesystem::path p(path.c_str());
+
+    try {
+        return boost::filesystem::exists(p);

Review comment:
       done




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli merged pull request #3741: Ignore broken disks when BE starts up

Posted by GitBox <gi...@apache.org>.
chaoyli merged pull request #3741:
URL: https://github.com/apache/incubator-doris/pull/3741


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org