Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add session pool #222

Merged
merged 10 commits into from
Oct 14, 2022
Merged

Add session pool #222

merged 10 commits into from
Oct 14, 2022

Conversation

Aiee
Copy link
Contributor

@Aiee Aiee commented Sep 6, 2022

Why we need a session pool

The purpose of adding the session pool is to solve the problem caused by improperly using the connection pool. For example, a common case is that the user generates a new session, executes a query, and releases the session in a loop, like:

// Wrong usage of the connection pool
for(int i=0; i<100; i++) {
  // get new session
  // execute query
  // release session
}

This will cause huge traffic in the meta service and may crash the service.

Usage

See session_pool_example.go for a more detailed example.

// Create configs for session pool
config, err := nebula.NewSessionPoolConf("root", "nebula", []nebula.HostAddress{hostAddress}, "example_space")

// create session pool
sessionPool, err := nebula.NewSessionPool(*config, nebula.DefaultLogger{})
defer sessionPool.Close()

// execute query
resultSet, err := sessionPool.Execute(stmt)

The usage of the connection pool remains unchanged.

Limitation

There are some limitations:

  1. There MUST be an existing space in the DB before initializing the session pool.
  2. Each session pool is corresponding to a single USER and a single Space. This is to ensure that the user's access control is consistent. i.g. The same user may have different access privileges in different spaces. If you need to run queries in different spaces, you may have multiple session pools.
  3. Every time when sessinPool.execute() is called, the session will execute the query in the space set in the session pool config.
  4. Commands that alter passwords or drop users should NOT be executed via session pool.

@Aiee Aiee added ready for testing wip Working in progress. doc affected PR: improvements or additions to documentation labels Sep 6, 2022
@Aiee Aiee added ready for review and removed wip Working in progress. labels Sep 16, 2022
@Aiee Aiee mentioned this pull request Sep 16, 2022
jievince
jievince previously approved these changes Sep 16, 2022
Copy link
Contributor

@jievince jievince left a comment

Choose a reason for hiding this comment

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

Good job

jievince
jievince previously approved these changes Sep 16, 2022
@akhilravuri-tul-scm
Copy link

akhilravuri-tul-scm commented Sep 19, 2022

Hi @Aiee

Thanks for developing this as it reduces session Aquire time.
I have tried to play around with this session pool.

package main

import (
	"fmt"
	"sync"
	"time"

	nebula "github.com/vesoft-inc/nebula-go/v3"
)

var wg1 sync.WaitGroup
var Pool1 *nebula.SessionPool

func DbCall(i int) {
	defer wg1.Done()
	resultSet, err := Pool1.Execute("FETCH PROP ON Person \"Bob\" YIELD properties(vertex) AS Result")
	if err != nil {
		fmt.Print(err.Error())
		return
	}
	fmt.Println("Go Routine: ", i, " : ", resultSet.AsStringTable())
}

// Initialize logger
var log = nebula.DefaultLogger{}

func main() {
	hostAddress := nebula.HostAddress{Host: "address", Port: 3669}

	// Create configs for session pool
	config, err := nebula.NewSessionPoolConf(
		nebula.WithUsername("root"),
		nebula.WithPassword("nebula"),
		nebula.WithServiceAddrs([]nebula.HostAddress{hostAddress}),
		nebula.WithSpaceName("example_space"),
		nebula.WithMaxSize(1000),
	)
	if err != nil {
		log.Fatal(fmt.Sprintf("failed to create session pool config, %s", err.Error()))
	}

	// create session pool
	Pool1, err = nebula.NewSessionPool(*config, nebula.DefaultLogger{})
	if err != nil {
		log.Fatal(fmt.Sprintf("failed to initialize session pool, %s", err.Error()))
	}
	defer Pool1.Close()
	t := time.Now()
	wg1.Add(4)
	go GoRoutine(0, 250)
	go GoRoutine(250, 500)
	go GoRoutine(500, 750)
	go GoRoutine(750, 1000)

	wg1.Wait()
	fmt.Println(time.Since(t))
}

func GoRoutine(start, end int) {
	defer wg1.Done()
	for i := start; i < end; i++ {
		wg1.Add(1)
		go DbCall(i)
	}
}

What I found was it takes 6sec to complete these 1k requests if nebula.WithMinSize(1000) is not provided.

So if if we use normal connection pool and getsession and execute for 1k req it takes 3sec.

Why it has doubled up the time as we just extended the connection pool?

And why time is getting reduced if we increase the MinSize is it because sessions are already created in the background?

And What is the ideal value for that MaxSize and MinSize?

Thanks,
Akhil

configs.go Outdated Show resolved Hide resolved
session_pool.go Outdated Show resolved Hide resolved
session_pool.go Outdated Show resolved Hide resolved
@Aiee Aiee added the wip Working in progress. label Sep 26, 2022
@akhilravuri-tul-scm
Copy link

@Aiee sessionPool.Execute() time is increasing as the number of vus increase. Using k6

Ex:- FETCH PROP ON Person "Bob" YIELD properties(vertex) AS Result

1 vus - P(90) is  3ms
10 vus - P(90) is 33ms
100 vus - p(90) is 304ms

Thanks,
Akhil

@Aiee
Copy link
Contributor Author

Aiee commented Oct 10, 2022

@Aiee sessionPool.Execute() time is increasing as the number of vus increase. Using k6

Ex:- FETCH PROP ON Person "Bob" YIELD properties(vertex) AS Result

1 vus - P(90) is  3ms
10 vus - P(90) is 33ms
100 vus - p(90) is 304ms

Thanks, Akhil

Hi @akhilravuri2,
Thank you for your feedback. The result seems reasonable to me because the query you run is relatively simple and the dataset is small, so the overhead of lock in the session pool will be a significant part of time consumption. I'll also add more benchmarks later to ensure the expected performance.

Also, the execute() method will create new sessions if there aren't any, so the time consumed by execute() includes the cost of building new sessions.

@akhilravuri-tul-scm
Copy link

akhilravuri-tul-scm commented Oct 10, 2022

@Aiee sessionPool.Execute() time is increasing as the number of vus increase. Using k6
Ex:- FETCH PROP ON Person "Bob" YIELD properties(vertex) AS Result

1 vus - P(90) is  3ms
10 vus - P(90) is 33ms
100 vus - p(90) is 304ms

Thanks, Akhil

Hi @akhilravuri2, Thank you for your feedback. The result seems reasonable to me because the query you run is relatively simple and the dataset is small, so the overhead of lock in the session pool will be a significant part of time consumption. I'll also add more benchmarks later to ensure the expected performance.

Also, the execute() method will create new sessions if there aren't any, so the time consumed by execute() includes the cost of building new sessions.

@Aiee Thanks for the response.
we have created enough sessions at the start of session pool and set it to never expire.(using maxSession and minsessions)
But session.Execute is taking linear increment with vus.
Is this behaviour expected as no new sessions are getting created?

If this is expected then how we can improve our response time for 100vus as 300ms is too much.

When you say overhead of locks is it about locks in session_pool.go at 120 Line.

Thanks,
Akhil

@Aiee
Copy link
Contributor Author

Aiee commented Oct 11, 2022

I added a benchmark in the session pool test and here is the result I got.

When you say overhead of locks is it about locks in session_pool.go at 120 Line.

Yes, I mean the locks being used when we move sessions from/to the list

> go test -benchmem -run=^$ -tags integration -bench ^BenchmarkConcurrency$ -v
goos: linux
goarch: amd64
pkg: github.com/vesoft-inc/nebula-go/v3
cpu: Intel(R) Xeon(R) Platinum 8352Y CPU @ 2.20GHz
BenchmarkConcurrency
BenchmarkConcurrency/5_clients
BenchmarkConcurrency/5_clients-128              1000000000               0.002056 ns/op        0 B/op          0 allocs/op
    session_pool_test.go:365: Concurrency: 5, Total time cost: 34.555572ms
BenchmarkConcurrency/10_clients
BenchmarkConcurrency/10_clients-128             1000000000               0.002122 ns/op        0 B/op          0 allocs/op
    session_pool_test.go:365: Concurrency: 10, Total time cost: 29.739571ms
BenchmarkConcurrency/100_clients
BenchmarkConcurrency/100_clients-128            1000000000               0.004205 ns/op        0 B/op          0 allocs/op
    session_pool_test.go:365: Concurrency: 100, Total time cost: 51.070175ms
BenchmarkConcurrency/1000_clients
BenchmarkConcurrency/1000_clients-128           1000000000               0.03122 ns/op         0 B/op          0 allocs/op
    session_pool_test.go:365: Concurrency: 1000, Total time cost: 283.493138ms
PASS
ok      github.com/vesoft-inc/nebula-go/v3      7.120s

@Aiee Aiee requested a review from HarrisChu October 11, 2022 10:57
Aiee added 3 commits October 11, 2022 19:00
@abby-cyber abby-cyber self-assigned this Oct 13, 2022
session_pool.go Show resolved Hide resolved
session_pool.go Show resolved Hide resolved
session_pool.go Show resolved Hide resolved
@Aiee Aiee requested a review from HarrisChu October 13, 2022 13:26
@Aiee Aiee removed the wip Working in progress. label Oct 14, 2022
@Aiee Aiee merged commit 82ab6c3 into master Oct 14, 2022
@Aiee Aiee deleted the session-pool branch October 14, 2022 02:41
@Sophie-Xie Sophie-Xie mentioned this pull request Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc affected PR: improvements or additions to documentation ready for review ready for testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants