enhance: resource estimate improvement #32964

chyezh · 2024-05-10T16:29:25Z

Add Exception to file class to avoid exception ignore.
Add GetResourceUsage method for index, column and segment.
Use bin log size but not mem size in CU of search and query.
Use in-use disk size in disk cache weight if segment is loaded.
Enable cache metric itself, remove redundant metric at search/query path.

sre-ci-robot · 2024-05-10T16:29:40Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chyezh
To complete the pull request process, please assign wxyucs after the PR has been reviewed.
You can assign the PR to them by writing /assign @wxyucs in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mergify · 2024-05-10T17:14:17Z

@chyezh ut workflow job failed, comment rerun ut can trigger the job again.

mergify · 2024-05-10T19:34:23Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-05-11T04:00:57Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-05-11T04:29:42Z

@chyezh ut workflow job failed, comment rerun ut can trigger the job again.

mergify · 2024-05-11T08:43:46Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-05-11T13:19:24Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

chyezh · 2024-05-11T15:29:06Z

/run-cpu-e2e

chyezh · 2024-05-11T15:29:11Z

rerun ut

mergify · 2024-05-11T16:14:33Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

chyezh · 2024-05-11T17:01:55Z

/run-cpu-e2e

codecov · 2024-05-11T17:17:10Z

Codecov Report

Attention: Patch coverage is 70.79646% with 99 lines in your changes are missing coverage. Please review.

Project coverage is 82.08%. Comparing base (8a9a421) to head (7935ad6).
Report is 28 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #32964    +/-   ##
========================================
  Coverage   82.07%   82.08%            
========================================
  Files        1007      999     -8     
  Lines      127647   128008   +361     
========================================
+ Hits       104767   105074   +307     
- Misses      18903    18960    +57     
+ Partials     3977     3974     -3

Files	Coverage Δ
internal/core/src/common/Types.h	`31.45% <ø> (+3.43%)`	⬆️
internal/core/src/index/Index.h	`100.00% <ø> (ø)`
internal/core/src/index/InvertedIndexTantivy.cpp	`89.47% <100.00%> (ø)`
internal/core/src/index/InvertedIndexTantivy.h	`47.36% <ø> (ø)`
internal/core/src/index/ScalarIndexSort.cpp	`74.63% <100.00%> (+0.21%)`	⬆️
internal/core/src/index/ScalarIndexSort.h	`57.14% <ø> (ø)`
internal/core/src/index/StringIndexMarisa.h	`100.00% <ø> (ø)`
internal/core/src/index/VectorDiskIndex.h	`18.18% <ø> (ø)`
internal/core/src/index/VectorMemIndex.h	`100.00% <ø> (ø)`
internal/core/src/segcore/SegmentGrowingImpl.h	`81.35% <100.00%> (+0.99%)`	⬆️
... and 31 more

... and 197 files with indirect coverage changes

mergify · 2024-05-12T06:25:17Z

@chyezh ut workflow job failed, comment rerun ut can trigger the job again.

mergify · 2024-05-12T08:44:04Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

Signed-off-by: chyezh <chyezh@outlook.com>

chyezh · 2024-05-12T12:52:53Z

rerun ut

chyezh · 2024-05-12T14:29:17Z

rerun ut

mergify · 2024-05-12T15:34:56Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

chyezh · 2024-05-12T15:35:57Z

/run-cpu-e2e

chyezh · 2024-05-12T16:21:10Z

rerun ut

chyezh · 2024-05-12T16:23:37Z

rerun ut

Signed-off-by: chyezh <chyezh@outlook.com>

mergify · 2024-05-12T19:26:17Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

chyezh · 2024-05-13T00:45:30Z

/run-cpu-e2e

sunby · 2024-05-13T02:37:40Z

/lgtm

mergify · 2024-05-13T03:44:17Z

@chyezh E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

chyezh · 2024-05-13T03:51:11Z

/run-cpu-e2e

longjiquan

lgtm generally for the cpp part.

longjiquan · 2024-05-13T06:13:39Z

internal/core/src/index/StringIndexMarisa.cpp

-        throw SegcoreError(
-            ErrorCode::UnistdError,
-            fmt::format("write index to fd error: {}", strerror(errno)));
+        throw;


throw; will rethrow the current exception.

longjiquan · 2024-05-13T06:21:20Z

internal/core/src/index/VectorMemIndex.cpp

                    write_disk_duration_sum +=
                        (std::chrono::system_clock::now() - start_write_file);
-                    AssertInfo(


Why ignore this exception here?

File instance in File.h is already handing the exception.
So exception handling can be ignored here.

longjiquan · 2024-05-13T06:30:12Z

internal/core/src/segcore/SegmentGrowingImpl.h

+    milvus::ResourceUsage
+    GetResourceUsage() const override {
+        auto mem_size = stats_.mem_size.load() + deleted_record_.mem_size();
+        return milvus::ResourceUsage{mem_size, 0};


Please also consider the intermediate index for growing index.

OK, it will be a TODO after incoming growing mmap index.

xiaofan-luan · 2024-05-13T07:08:21Z

internal/core/src/index/StringIndexMarisa.cpp

+ResourceUsage
+StringIndexMarisa::GetResourceUsage() const {
+    // TODO: we cannot estimate the memory usage of marisa trie.
+    return resource_usage_;


die we enable mmap for marisa trie?
if so, the memory usage can be set to 0

xiaofan-luan · 2024-05-13T07:09:42Z

internal/core/src/index/VectorDiskIndex.cpp

+VectorDiskAnnIndex<T>::GetResourceUsage() const {
+    // TODO: we can't get the memory usage of vector index now.
+    auto disk_size = file_manager_->GetLocalFileSize();
+    return ResourceUsage{0, disk_size};


this need to double check with @liliu-z
i believe there is a way to estimate memory utilizations

xiaofan-luan · 2024-05-13T07:14:22Z

internal/core/src/index/VectorMemIndex.cpp

+ResourceUsage
+VectorMemIndex<T>::GetResourceUsage() const {
+    // TODO: we can't get the memory usage of vector index now.
+    return resource_usage_;


if mmap is not enbaled, resource usage should be all memory usage. but I dind't see any calculation here.

Ok, I will use file size as mem size if disable mmap

xiaofan-luan · 2024-05-13T07:31:52Z

internal/querynodev2/segments/metricsutil/observer.go

@@ -179,12 +167,6 @@ type promMetricsObserver struct {
 	nodeID string
 	label  SegmentLabel // never updates

-	DiskCacheLoadTotal                   prometheus.Counter


why we remove these metrics?

There's already exported in cache monitor.go
repeated metrics can be removed.

xiaofan-luan · 2024-05-13T07:56:13Z

internal/querynodev2/segments/segment_interface.go

-	// ResourceUsageEstimate returns the estimated resource usage of the segment
-	ResourceUsageEstimate() ResourceUsage
+
+	// ResourceUsageEstimate returns the estimated resource usage of the segment.


change the variable name,
ResourceUsageEstimateofLoad

xiaofan-luan · 2024-05-13T08:17:46Z

internal/querynodev2/segments/segment_l0.go

 		return pk.Size() + 8
 	})
+	return SegmentResourceUsage{
+		Predict: ResourceUsage{


how do you get predict value before you load it?

original segment loader logic is used in prediction.

xiaofan-luan · 2024-05-13T08:37:57Z

internal/querynodev2/segments/manager.go

@@ -180,7 +180,7 @@ func NewManager() *Manager {
 		if segment == nil {
 			return 0
 		}
-		return int64(segment.ResourceUsageEstimate().DiskSize)
+		return int64(segment.ResourceUsageEstimate().GetInuseOrPredictDiskUsage())


for cache, if weight is set.
once we use index as alternative, or add new delete, the weight will not be correct.

xiaofan-luan · 2024-05-13T08:42:57Z

internal/querynodev2/segments/segment_loader.go

@@ -1540,7 +1540,7 @@ func (loader *segmentLoader) checkSegmentSize(ctx context.Context, segmentLoadIn
 			zap.Float64("diskUsage(MB)", toMB(usage.DiskSize)),
 			zap.Float64("memoryLoadFactor", factor.memoryUsageFactor),
 		)
-		mmapFieldCount += usage.MmapFieldCount
+		mmapFieldCount += mmapFieldCount


how does we use mmapFieldCount?
and mmapFieldCount += mmapFieldCount seems to be a bug

only used in logging, record how many fields to be mmap.

xiaofan-luan · 2024-05-13T08:48:14Z

pkg/util/cache/cache.go

 }

 type LazyScavenger[K comparable] struct {
 	capacity int64
-	size     int64
+	size     *atomic.Int64
 	weight   func(K) int64
 	weights  map[K]int64


do not maintain weights here.
we should cache sizable object

sre-ci-robot added the size/XL Denotes a PR that changes 500-999 lines. label May 10, 2024

sre-ci-robot requested review from aoiasd and bigsheeper May 10, 2024 16:29

mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels May 10, 2024

chyezh force-pushed the fix_lru_master branch from 10ac56b to 1d86bc3 Compare May 11, 2024 03:05

sre-ci-robot added size/XXL Denotes a PR that changes 1000+ lines. and removed size/XL Denotes a PR that changes 500-999 lines. labels May 11, 2024

chyezh force-pushed the fix_lru_master branch 2 times, most recently from d774a06 to 5c8d83c Compare May 11, 2024 03:19

chyezh force-pushed the fix_lru_master branch from bb1a54d to f4624b5 Compare May 11, 2024 07:40

chyezh force-pushed the fix_lru_master branch 2 times, most recently from 46989a0 to 595b74c Compare May 11, 2024 12:45

chyezh force-pushed the fix_lru_master branch 2 times, most recently from f37ae6f to dd77db4 Compare May 12, 2024 05:40

fix: add assertion on File

70275d9

Signed-off-by: chyezh <chyezh@outlook.com>

chyezh added 4 commits May 12, 2024 18:51

enhance: add GetResourceUsage for index interface

d80938b

Signed-off-by: chyezh <chyezh@outlook.com>

enhance: add GetResourceUsage for column interface and Segment

b3d1509

Signed-off-by: chyezh <chyezh@outlook.com>

enhance: resource estimate improvement

8485486

Signed-off-by: chyezh <chyezh@outlook.com>

feature: use in-use disk at disk cache and add metrics

a4c554b

Signed-off-by: chyezh <chyezh@outlook.com>

chyezh force-pushed the fix_lru_master branch from dd77db4 to 6bb1122 Compare May 12, 2024 10:56

chyezh force-pushed the fix_lru_master branch from 6bb1122 to d26cd18 Compare May 12, 2024 14:31

fix: remove redundant lru cache metrics

7935ad6

Signed-off-by: chyezh <chyezh@outlook.com>

chyezh force-pushed the fix_lru_master branch from d26cd18 to 7935ad6 Compare May 12, 2024 16:24

sre-ci-robot assigned sunby May 13, 2024

sre-ci-robot added the lgtm label May 13, 2024

chyezh added this to the 2.4.2 milestone May 13, 2024

longjiquan reviewed May 13, 2024

View reviewed changes

xiaofan-luan reviewed May 13, 2024

View reviewed changes

chyezh added the do-not-merge/hold label May 14, 2024

czs007 removed this from the 2.4.2 milestone May 14, 2024

enhance: resource estimate improvement #32964

Are you sure you want to change the base?

enhance: resource estimate improvement #32964

Conversation

chyezh commented May 10, 2024 • edited

sre-ci-robot commented May 10, 2024

mergify bot commented May 10, 2024

mergify bot commented May 10, 2024

mergify bot commented May 11, 2024

mergify bot commented May 11, 2024

mergify bot commented May 11, 2024

mergify bot commented May 11, 2024

chyezh commented May 11, 2024

chyezh commented May 11, 2024

mergify bot commented May 11, 2024

chyezh commented May 11, 2024

codecov bot commented May 11, 2024 • edited

Codecov Report

mergify bot commented May 12, 2024

mergify bot commented May 12, 2024

chyezh commented May 12, 2024

chyezh commented May 12, 2024

mergify bot commented May 12, 2024

chyezh commented May 12, 2024

chyezh commented May 12, 2024

chyezh commented May 12, 2024

mergify bot commented May 12, 2024

chyezh commented May 13, 2024

sunby commented May 13, 2024

mergify bot commented May 13, 2024

chyezh commented May 13, 2024

longjiquan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chyezh commented May 10, 2024 •

edited

codecov bot commented May 11, 2024 •

edited