fix: Fix binlog import (#31310)

Fix binlog import functionality by removing the existing check and
refining the size retrieval process.

issue: https://github.com/milvus-io/milvus/issues/31221,
https://github.com/milvus-io/milvus/issues/28521

---------

Signed-off-by: bigsheeper <yihao.dai@zilliz.com>
pull/31255/head
yihao.dai 2024-03-17 20:59:04 +08:00 committed by GitHub
parent e396ad9580
commit 776709e5ff
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 20 additions and 70 deletions

View File

@ -18,7 +18,6 @@ package datacoord
import ( import (
"context" "context"
"fmt"
"path" "path"
"sort" "sort"
"time" "time"
@ -424,13 +423,6 @@ func ListBinlogsAndGroupBySegment(ctx context.Context, cm storage.ChunkManager,
} }
insertPrefix := importFile.GetPaths()[0] insertPrefix := importFile.GetPaths()[0]
ok, err := cm.Exist(ctx, insertPrefix)
if err != nil {
return nil, err
}
if !ok {
return nil, fmt.Errorf("insert binlog prefix does not exist, path=%s", insertPrefix)
}
segmentInsertPaths, _, err := cm.ListWithPrefix(ctx, insertPrefix, false) segmentInsertPaths, _, err := cm.ListWithPrefix(ctx, insertPrefix, false)
if err != nil { if err != nil {
return nil, err return nil, err
@ -443,14 +435,6 @@ func ListBinlogsAndGroupBySegment(ctx context.Context, cm storage.ChunkManager,
return segmentImportFiles, nil return segmentImportFiles, nil
} }
deltaPrefix := importFile.GetPaths()[1] deltaPrefix := importFile.GetPaths()[1]
ok, err = cm.Exist(ctx, deltaPrefix)
if err != nil {
return nil, err
}
if !ok {
log.Warn("delta binlog prefix does not exist", zap.String("path", deltaPrefix))
return segmentImportFiles, nil
}
segmentDeltaPaths, _, err := cm.ListWithPrefix(context.Background(), deltaPrefix, false) segmentDeltaPaths, _, err := cm.ListWithPrefix(context.Background(), deltaPrefix, false)
if err != nil { if err != nil {
return nil, err return nil, err

View File

@ -336,7 +336,6 @@ func TestImportUtil_ListBinlogsAndGroupBySegment(t *testing.T) {
ctx := context.Background() ctx := context.Background()
cm := mocks2.NewChunkManager(t) cm := mocks2.NewChunkManager(t)
cm.EXPECT().Exist(mock.Anything, mock.Anything).Return(true, nil)
cm.EXPECT().ListWithPrefix(mock.Anything, insertPrefix, mock.Anything).Return(segmentInsertPaths, nil, nil) cm.EXPECT().ListWithPrefix(mock.Anything, insertPrefix, mock.Anything).Return(segmentInsertPaths, nil, nil)
cm.EXPECT().ListWithPrefix(mock.Anything, deltaPrefix, mock.Anything).Return(segmentDeltaPaths, nil, nil) cm.EXPECT().ListWithPrefix(mock.Anything, deltaPrefix, mock.Anything).Return(segmentDeltaPaths, nil, nil)
@ -355,32 +354,6 @@ func TestImportUtil_ListBinlogsAndGroupBySegment(t *testing.T) {
assert.True(t, segmentID == "435978159261483008" || segmentID == "435978159261483009") assert.True(t, segmentID == "435978159261483008" || segmentID == "435978159261483009")
} }
} }
// test failure
mockErr := errors.New("mock err")
cm = mocks2.NewChunkManager(t)
cm.EXPECT().Exist(mock.Anything, insertPrefix).Return(true, mockErr)
_, err = ListBinlogsAndGroupBySegment(ctx, cm, file)
assert.Error(t, err)
cm = mocks2.NewChunkManager(t)
cm.EXPECT().Exist(mock.Anything, insertPrefix).Return(false, nil)
_, err = ListBinlogsAndGroupBySegment(ctx, cm, file)
assert.Error(t, err)
cm = mocks2.NewChunkManager(t)
cm.EXPECT().Exist(mock.Anything, insertPrefix).Return(true, nil)
cm.EXPECT().ListWithPrefix(mock.Anything, insertPrefix, mock.Anything).Return(segmentInsertPaths, nil, nil)
cm.EXPECT().Exist(mock.Anything, deltaPrefix).Return(true, mockErr)
_, err = ListBinlogsAndGroupBySegment(ctx, cm, file)
assert.Error(t, err)
cm = mocks2.NewChunkManager(t)
cm.EXPECT().Exist(mock.Anything, insertPrefix).Return(true, nil)
cm.EXPECT().ListWithPrefix(mock.Anything, insertPrefix, mock.Anything).Return(segmentInsertPaths, nil, nil)
cm.EXPECT().Exist(mock.Anything, deltaPrefix).Return(false, nil)
_, err = ListBinlogsAndGroupBySegment(ctx, cm, file)
assert.NoError(t, err)
} }
func TestImportUtil_GetImportProgress(t *testing.T) { func TestImportUtil_GetImportProgress(t *testing.T) {

View File

@ -1808,9 +1808,10 @@ func (s *Server) ImportV2(ctx context.Context, in *internalpb.ImportRequestInter
return len(file.GetPaths()) > 0 return len(file.GetPaths()) > 0
}) })
if len(files) == 0 { if len(files) == 0 {
resp.Status = merr.Status(merr.WrapErrParameterInvalidMsg(fmt.Sprintf("no binlog to import, import_prefix=%s", in.GetFiles()))) resp.Status = merr.Status(merr.WrapErrParameterInvalidMsg(fmt.Sprintf("no binlog to import, input=%s", in.GetFiles())))
return resp, nil return resp, nil
} }
log.Info("list binlogs prefixes for import", zap.Any("binlog_prefixes", files))
} }
idStart, _, err := s.allocator.allocN(int64(len(files)) + 1) idStart, _, err := s.allocator.allocN(int64(len(files)) + 1)

View File

@ -1369,7 +1369,6 @@ func TestImportV2(t *testing.T) {
// list binlog failed // list binlog failed
cm := mocks2.NewChunkManager(t) cm := mocks2.NewChunkManager(t)
cm.EXPECT().Exist(mock.Anything, mock.Anything).Return(true, nil)
cm.EXPECT().ListWithPrefix(mock.Anything, mock.Anything, mock.Anything).Return(nil, nil, mockErr) cm.EXPECT().ListWithPrefix(mock.Anything, mock.Anything, mock.Anything).Return(nil, nil, mockErr)
s.meta = &meta{chunkManager: cm} s.meta = &meta{chunkManager: cm}
resp, err = s.ImportV2(ctx, &internalpb.ImportRequestInternal{ resp, err = s.ImportV2(ctx, &internalpb.ImportRequestInternal{
@ -1389,28 +1388,6 @@ func TestImportV2(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, errors.Is(merr.Error(resp.GetStatus()), merr.ErrImportFailed)) assert.True(t, errors.Is(merr.Error(resp.GetStatus()), merr.ErrImportFailed))
// list no binlog
cm = mocks2.NewChunkManager(t)
cm.EXPECT().Exist(mock.Anything, mock.Anything).Return(true, nil)
cm.EXPECT().ListWithPrefix(mock.Anything, mock.Anything, mock.Anything).Return(nil, nil, nil)
s.meta = &meta{chunkManager: cm}
resp, err = s.ImportV2(ctx, &internalpb.ImportRequestInternal{
Files: []*internalpb.ImportFile{
{
Id: 1,
Paths: []string{"mock_insert_prefix"},
},
},
Options: []*commonpb.KeyValuePair{
{
Key: "backup",
Value: "true",
},
},
})
assert.NoError(t, err)
assert.True(t, errors.Is(merr.Error(resp.GetStatus()), merr.ErrParameterInvalid))
// alloc failed // alloc failed
alloc := NewNMockAllocator(t) alloc := NewNMockAllocator(t)
alloc.EXPECT().allocN(mock.Anything).Return(0, 0, mockErr) alloc.EXPECT().allocN(mock.Anything).Return(0, 0, mockErr)

View File

@ -181,7 +181,7 @@ func (e *executor) PreImport(task Task) {
} }
func (e *executor) readFileStat(reader importutilv2.Reader, task Task, fileIdx int, file *internalpb.ImportFile) error { func (e *executor) readFileStat(reader importutilv2.Reader, task Task, fileIdx int, file *internalpb.ImportFile) error {
fileSize, err := GetFileSize(file, e.cm) fileSize, err := GetFileSize(file, e.cm, task)
if err != nil { if err != nil {
return err return err
} }

View File

@ -33,6 +33,7 @@ import (
"github.com/milvus-io/milvus/internal/proto/internalpb" "github.com/milvus-io/milvus/internal/proto/internalpb"
"github.com/milvus-io/milvus/internal/querycoordv2/params" "github.com/milvus-io/milvus/internal/querycoordv2/params"
"github.com/milvus-io/milvus/internal/storage" "github.com/milvus-io/milvus/internal/storage"
"github.com/milvus-io/milvus/internal/util/importutilv2"
"github.com/milvus-io/milvus/pkg/common" "github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/log" "github.com/milvus-io/milvus/pkg/log"
"github.com/milvus-io/milvus/pkg/util/merr" "github.com/milvus-io/milvus/pkg/util/merr"
@ -203,14 +204,28 @@ func GetInsertDataRowCount(data *storage.InsertData, schema *schemapb.Collection
return 0 return 0
} }
func GetFileSize(file *internalpb.ImportFile, cm storage.ChunkManager) (int64, error) { func GetFileSize(file *internalpb.ImportFile, cm storage.ChunkManager, task Task) (int64, error) {
paths := file.GetPaths()
if importutilv2.IsBackup(task.GetOptions()) {
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
defer cancel()
paths = make([]string, 0)
for _, prefix := range file.GetPaths() {
binlogs, _, err := cm.ListWithPrefix(ctx, prefix, true)
if err != nil {
return 0, err
}
paths = append(paths, binlogs...)
}
}
fn := func(path string) (int64, error) { fn := func(path string) (int64, error) {
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
defer cancel() defer cancel()
return cm.Size(ctx, path) return cm.Size(ctx, path)
} }
var totalSize int64 = 0 var totalSize int64 = 0
for _, path := range file.GetPaths() { for _, path := range paths {
size, err := fn(path) size, err := fn(path)
if err != nil { if err != nil {
return 0, err return 0, err