Skip to content

Commit

Permalink
[enhance](Backup) Do connectivity check when creating repository (#38350
Browse files Browse the repository at this point in the history
) (#39538)

Previously when creating repository, FE would not do connectivity check.
It might result in confusing error when using backup restore.

pick #38350

Co-authored-by: AlexYue <yj976240184@gmail.com>
  • Loading branch information
gavinchou and ByteYue authored Aug 19, 2024
1 parent 3922fdd commit 621d394
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ public class BackupHandler extends MasterDaemon implements Writable {
public static final int SIGNATURE_VERSION = 1;
public static final Path BACKUP_ROOT_DIR = Paths.get(Config.tmp_dir, "backup").normalize();
public static final Path RESTORE_ROOT_DIR = Paths.get(Config.tmp_dir, "restore").normalize();

private RepositoryMgr repoMgr = new RepositoryMgr();

// this lock is used for updating dbIdToBackupOrRestoreJobs
Expand Down Expand Up @@ -220,6 +219,10 @@ public void createRepository(CreateRepositoryStmt stmt) throws DdlException {
ErrorReport.reportDdlException(ErrorCode.ERR_COMMON_ERROR,
"Failed to create repository: " + st.getErrMsg());
}
if (!repo.ping()) {
ErrorReport.reportDdlException(ErrorCode.ERR_COMMON_ERROR,
"Failed to create repository: failed to connect to the repo");
}
}

public void alterRepository(AlterRepositoryStmt stmt) throws DdlException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,9 @@ public String getRepoPath(String label, String childPath) {
// Check if this repo is available.
// If failed to connect this repo, set errMsg and return false.
public boolean ping() {
if (FeConstants.runningUnitTest) {
return true;
}
// for s3 sdk, the headObject() method does not support list "dir",
// so we check FILE_REPO_INFO instead.
String path = location + "/" + joinPrefix(PREFIX_REPO, name) + "/" + FILE_REPO_INFO;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public void setUp() throws Exception {
Config.tmp_dir = tmpPath;
rootDir = new File(Config.tmp_dir);
rootDir.mkdirs();
FeConstants.runningUnitTest = true;

new Expectations() {
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// 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.

suite("test_backup_restore", "connectivity_failed") {
def name = "test_name"
def bucket = "useless_bucket"
def prefix = "useless_prefix"
def endpoint = getS3Endpoint()
def region = getS3Region()
def ak = getS3AK()
def sk = getS3SK()
expectExceptionLike({
sql """
CREATE REPOSITORY `${name}`
WITH S3
ON LOCATION "s3://${bucket}/${prefix}/${name}"
PROPERTIES
(
"s3.endpoint" = "http://${endpoint}",
"s3.region" = "${region}",
"s3.access_key" = "${ak}",
"s3.secret_key" = "${sk}"
)
"""
}, "Failed to create repository")
}

0 comments on commit 621d394

Please sign in to comment.