statsMgr information get for studio #194

xjlgod · 2022-01-26T07:15:09Z

At present, Nebula Studio is doing multi task import. It need to know the total number of imported lines and the number of imported lines of each task.

CLAassistant · 2022-01-26T07:15:14Z

All committers have signed the CLA.

veezhang · 2022-01-26T08:03:38Z

.idea/.gitignore

@@ -0,0 +1,8 @@
+# Default ignored files


please remove files in .idea directory.

veezhang · 2022-01-26T08:25:53Z

pkg/stats/statsmgr.go

+	var wg sync.WaitGroup
+	wg.Add(numReadingFiles)
+	for _, file := range files {
+		path := file.FailDataPath
+		go func(path string) {
+			totalLine += countLines(path)
+			defer wg.Done()
+		}(*path)
+	}
+	wg.Wait()


There seems to be no need for multiple goroutine here?

veezhang · 2022-01-26T08:26:53Z

pkg/stats/statsmgr.go

@@ -92,3 +133,16 @@ func (s *StatsMgr) startWorker(numReadingFiles int) {
 		}
 	}
 }
+
+func countLines(path string) int64 {


This will cause the file to be read twice, will it affect performance?

Will the size of the file be considered, not the number of lines in the file? Add TotalSize, ProcessedSize?

veezhang · 2022-01-26T08:27:35Z

pkg/stats/statsmgr.go

+func countLines(path string) int64 {
+	file, err := os.Open(path)
+	if err != nil {
+		return 0


Don't eat err?

veezhang · 2022-01-26T08:42:42Z

pkg/stats/statsmgr.go

 	totalCount    int64
 	totalBatches  int64
 	totalLatency  int64
 	totalReqTime  int64
 }

-func NewStatsMgr(numReadingFiles int) *StatsMgr {
+type StatsQuery struct {


How about define StatsMgr as follows?

type StatsMgr struct { StatsCh chan base.Stats DoneCh chan bool statsRW sync.RWMutex stats Stats } type Stats struct { NumFailed int64 NumReadFailed int64 TotalLine int64 TotalCount int64 TotalBatches int64 TotalLatency int64 TotalReqTime int64 }

Then, you can define a function func (s *StatsMgr) Stats() Stats replace the current StatsQuery function.

jievince

Good job!

jievince · 2022-01-26T09:54:31Z

.gitignore

@@ -21,3 +21,4 @@ nebula-importer

 # IDE
 .vscode/
+.idea/


Please configure your ide/editor to auto add a new blank line at the end of a file

jievince · 2022-01-26T10:04:08Z

pkg/stats/statsmgr.go

+	lineCount := int64(0)
+	for fileScanner.Scan() {
+		lineCount++
+	}


Does it is faster? https://stackoverflow.com/a/24563853

veezhang · 2022-02-22T06:06:58Z

pkg/stats/statsmgr.go

+	// There is no \n in the last line
+	count := 1


How do you know?
Maybe ?

a b

or

a b

veezhang · 2022-02-22T07:03:28Z

pkg/stats/statsmgr.go

@@ -92,3 +139,27 @@ func (s *StatsMgr) startWorker(numReadingFiles int) {
 		}
 	}
 }
+
+func countLines(path string) (int64, error) {


Should these be considered?

type CSVConfig struct { WithHeader *bool `json:"withHeader" yaml:"withHeader"` WithLabel *bool `json:"withLabel" yaml:"withLabel"` Delimiter *string `json:"delimiter" yaml:"delimiter"` }

https://github.com/vesoft-inc/nebula-importer/blob/master/pkg/config/config.go#L86-L90

@jievince How do you think?

veezhang

LGTM

yixinglu · 2022-03-01T04:10:41Z

pkg/stats/statsmgr.go

+		path := file.Path
+		withHeader := file.CSV.WithHeader
+		go func(path string) {
+			lines, err := csv.CountLines(path, *withHeader)


I think counting file lines is impractical for huge file in new coroutine. Why not to do this when reading these data files?

xjlgod · 2022-03-01T05:39:59Z

Because Studio needs to know in advance how many lines the file needs to be imported , before importing.

veezhang · 2022-03-03T10:56:12Z

pkg/csv/reader.go

+}
+
+func CountFileBytes(path string, withHeader bool) (int64, error) {
+	file, err := os.Open(path)


The statistical methods of the two functions are different.

countLineBytes is the sum of bytes of the csv field.

CountFileBytes is to get the file size and then subtract the header bytes.

You can take statistics bytes and don't care about headers.

countLineBytes get the bytes read.

CountFileBytes get file size.

var _ io.Reader = (*recordReader)(nil) type recordReader struct { io.Reader lastReadBytes int } func (r *recordReader) Read(p []byte) (n int, err error) { n, err = r.Reader.Read(p) r.lastReadBytes = n return } func (r *CSVReader) InitReader(file *os.File) { r.rr = &recordReader{Reader: bufio.NewReader(file)} r.reader = csv.NewReader(r.rr) ... } func (r *CSVReader) ReadLine() (base.Data, error) { line, err := r.reader.Read() // r.rr.lastReadBytes // get the last read bytes ... }

veezhang · 2022-03-08T10:40:42Z

pkg/base/stats.go

+	Latency       int64
+	ReqTime       int64
+	BatchSize     int
+	ImportedBytesNum int64


How about use ImportedBytes ?

veezhang · 2022-03-08T10:41:26Z

pkg/base/types.go

@@ -34,26 +34,30 @@ func (op OpType) String() string {
 type Data struct {
 	Type   OpType
 	Record Record
+	BytesNum int


How about use Bytes ?

veezhang

LGTM

yixinglu · 2022-03-09T07:56:06Z

pkg/stats/statsmgr.go

-	s.totalCount += int64(stat.BatchSize)
-	s.totalReqTime += stat.ReqTime
-	s.totalLatency += stat.Latency
+	s.statsRW.Lock()


Why do we need this lock? Are these stats not be sent by one channel?

yixinglu · 2022-03-09T07:56:57Z

please sign the CLA firstly!

xjlgod · 2022-03-09T08:15:40Z

please sign the CLA firstly!

Ok, I hava signed it.

veezhang · 2022-03-10T02:01:28Z

It's need not to use reflect as follows:

package main

import (
	"bufio"
	"encoding/csv"
	"fmt"
	"io"
	"os"
)

type ProgressReader struct {
	r              io.Reader
	totalBytes     int64
	bytes          int64
	remainingBytes int64
}

func NewProgressReader(totalBytes int64, r io.Reader) *ProgressReader {
	if totalBytes <= 0 {
		panic("the totalBytes must greater than 0")
	}
	return &ProgressReader{
		r:          r,
		totalBytes: totalBytes,
	}
}

func (r *ProgressReader) Read(p []byte) (n int, err error) {
	n, err = r.r.Read(p)
	r.bytes += int64(n)
	r.remainingBytes += int64(n)
	return n, err
}

func (r *ProgressReader) GetCurrentBytes(offset int64) int64 {
	n := r.remainingBytes + offset
	r.remainingBytes -= n
	return n
}

func (r *ProgressReader) Percentage() float64 {
	return 100 * float64(r.bytes) / float64(r.totalBytes)
}

func (r *ProgressReader) PercentageOffset(offset int64) float64 {
	return 100 * float64(r.bytes+offset) / float64(r.totalBytes)
}

func (r *ProgressReader) Bytes() int64 {
	return r.bytes
}

func main() {
	f, err := os.Open("/Users/veezhang/Downloads/annual-enterprise-survey-2020-financial-year-provisional-csv.csv")
	if err != nil {
		panic(err)
	}
	stat, err := f.Stat()
	if err != nil {
		panic(err)
	}

	totalBytes := stat.Size()
	pr := NewProgressReader(totalBytes, f)
	br := bufio.NewReader(pr)
	r := csv.NewReader(br)

	for {
		_, err := r.Read()
		if err != nil {
			if err == io.EOF {
				break
			}
			panic(err)
		}

		fmt.Printf(
			"%.3f %.3f %d %d\n",
			pr.Percentage(),
			pr.PercentageOffset(int64(-br.Buffered())),
			pr.GetCurrentBytes(int64(-br.Buffered())),
			-br.Buffered(),
		)
	}
}

The output :

...
100.000 99.979 192 -1245
100.000 99.982 186 -1059
100.000 99.985 170 -889
100.000 99.988 168 -721
100.000 99.991 192 -529
100.000 99.994 173 -356
100.000 99.997 178 -178
100.000 100.000 178 0

veezhang · 2022-03-11T01:24:20Z

pkg/csv/reader.go

 	"github.com/vesoft-inc/nebula-importer/pkg/base"
 	"github.com/vesoft-inc/nebula-importer/pkg/config"
 	"github.com/vesoft-inc/nebula-importer/pkg/logger"
+	"io"
+	"os"


veezhang · 2022-03-11T01:25:51Z

pkg/csv/reader.go

+func (r *CSVReader) TotalBytes() (int64) {
+	for {
+		if r.initComplete {
+			return r.totalBytes
+		}


It's inappropriate?

veezhang · 2022-03-11T01:39:35Z

pkg/cmd/runner.go

+		if r.Readers == nil {
+			return &r.stataMgr.Stats
+		}
+		r.stataMgr.StatsCh <- base.NewOutputStats()


Why add chan when it can be read directly?

veezhang · 2022-03-11T01:41:12Z

pkg/cmd/runner.go

+
+func (r *Runner) QueryStats() *stats.Stats {
+	if r.stataMgr != nil {
+		if r.Readers == nil {


Why need to check the r.Readers?

veezhang · 2022-03-11T01:45:52Z

pkg/cmd/runner.go

+	}
+}
+
+func (r *Runner) QueryStats() *stats.Stats {


How about return stats.Stats nor *stats.Stats?

veezhang

LGTM

veezhang reviewed Jan 26, 2022

View reviewed changes

jievince reviewed Jan 26, 2022

View reviewed changes

jievince marked this pull request as ready for review January 26, 2022 10:06

jievince requested a review from yixinglu as a code owner January 26, 2022 10:06

veezhang self-requested a review February 21, 2022 06:53

veezhang reviewed Feb 22, 2022

View reviewed changes

xjlgod force-pushed the dev branch from f134df7 to bf87ee7 Compare February 28, 2022 05:51

veezhang previously approved these changes Feb 28, 2022

View reviewed changes

yixinglu reviewed Mar 1, 2022

View reviewed changes

xjlgod dismissed veezhang’s stale review via 291daca March 2, 2022 07:25

veezhang reviewed Mar 3, 2022

View reviewed changes

veezhang reviewed Mar 8, 2022

View reviewed changes

xjlgod force-pushed the dev branch from 6ce7fdd to 91f7832 Compare March 9, 2022 02:24

veezhang previously approved these changes Mar 9, 2022

View reviewed changes

yixinglu reviewed Mar 9, 2022

View reviewed changes

xjlgod dismissed veezhang’s stale review via 3c0ffe2 March 9, 2022 08:28

xjlgod force-pushed the dev branch from 3c0ffe2 to 7ed0efb Compare March 9, 2022 08:52

statsMgr information get for studio

9ced538

xjlgod force-pushed the dev branch from 7ed0efb to 9ced538 Compare March 9, 2022 08:53

yixinglu previously approved these changes Mar 9, 2022

View reviewed changes

Merge branch 'master' into dev

8a6d7ef

xjlgod added 2 commits March 10, 2022 11:29

del reflect and fix ci

031d009

Merge branch 'dev' of https://github.com/xjlgod/nebula-importer into dev

7d27704

xjlgod dismissed yixinglu’s stale review via 7d27704 March 10, 2022 03:33

fix totalByte

ca82699

veezhang reviewed Mar 11, 2022

View reviewed changes

pkg/cmd/runner.go Outdated

}

}

func (r *Runner) QueryStats() *stats.Stats {

Copy link

Contributor

veezhang Mar 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about return stats.Stats nor *stats.Stats?

optimize count file bytes and isDone

b15671b

xjlgod force-pushed the dev branch from 06d7eb2 to b15671b Compare March 11, 2022 03:07

xjlgod added 2 commits March 11, 2022 11:32

optimize count file bytes and query stats

96a5e78

fix total count

51d0359

veezhang approved these changes Mar 14, 2022

View reviewed changes

yixinglu approved these changes Mar 14, 2022

View reviewed changes

yixinglu merged commit ece1a79 into vesoft-inc:master Mar 14, 2022

jamieliu1023 mentioned this pull request Mar 19, 2022

Weekly Report 2022-03-18 vesoft-inc/nebula-community#100

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

statsMgr information get for studio #194

statsMgr information get for studio #194

xjlgod commented Jan 26, 2022

CLAassistant commented Jan 26, 2022 •

edited

Loading

veezhang Jan 26, 2022

veezhang Jan 26, 2022

veezhang Jan 26, 2022

veezhang Jan 26, 2022

veezhang Jan 26, 2022

veezhang Jan 26, 2022

jievince left a comment

jievince Jan 26, 2022

jievince Jan 26, 2022

veezhang Feb 22, 2022

veezhang Feb 22, 2022 •

edited

Loading

veezhang Feb 22, 2022

veezhang left a comment

yixinglu Mar 1, 2022

xjlgod commented Mar 1, 2022

veezhang Mar 3, 2022 •

edited

Loading

veezhang Mar 3, 2022

veezhang Mar 8, 2022

veezhang Mar 8, 2022

veezhang left a comment

yixinglu Mar 9, 2022

yixinglu commented Mar 9, 2022

xjlgod commented Mar 9, 2022

veezhang commented Mar 10, 2022 •

edited

Loading

veezhang Mar 11, 2022

veezhang Mar 11, 2022

veezhang Mar 11, 2022

veezhang Mar 11, 2022

veezhang Mar 11, 2022

veezhang left a comment

statsMgr information get for studio #194

statsMgr information get for studio #194

Conversation

xjlgod commented Jan 26, 2022

CLAassistant commented Jan 26, 2022 • edited Loading

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

jievince 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

veezhang Feb 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veezhang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xjlgod commented Mar 1, 2022

veezhang Mar 3, 2022 • edited Loading

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

veezhang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yixinglu commented Mar 9, 2022

xjlgod commented Mar 9, 2022

veezhang commented Mar 10, 2022 • edited Loading

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

veezhang left a comment

Choose a reason for hiding this comment

CLAassistant commented Jan 26, 2022 •

edited

Loading

veezhang Feb 22, 2022 •

edited

Loading

veezhang Mar 3, 2022 •

edited

Loading

veezhang commented Mar 10, 2022 •

edited

Loading