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 active Close method. #43

Closed
ashutosh-akss opened this issue Jun 18, 2017 · 7 comments
Closed

Add active Close method. #43

ashutosh-akss opened this issue Jun 18, 2017 · 7 comments

Comments

@ashutosh-akss
Copy link

There is no way to close a connection and it's resulting in memory leak
my 8gb RAM is filled within 1.5 minutes

Get http://localhost:8529/_db/redbus/_api/database/current: dial tcp 127.0.0.1:8529: connect: cannot assign requested address
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x70 pc=0x4bc8be]

I am doing something like this

for _,item := range SomeArray{
ic:=getCount(item)
}

func getCount(Item) (iCount int) {

conn, err := http.NewConnection(http.ConnectionConfig{
	Endpoints: []string{"http://localhost:8529"},
})
if err != nil {
	fmt.Println("Error in Arango Connection", err)
} else {
	// fmt.Println("Arangodb connection successfull")
}
c, err := driver.NewClient(driver.ClientConfig{
	Connection:     conn,
	Authentication: driver.BasicAuthentication("test", "test"),
})

if err != nil {
	fmt.Println("Error in Arango Client Creation", err)
} else {
	fmt.Println("Arangodb client creation successful")
}
db, err := c.Database(nil, "redbus")
if err != nil {
	fmt.Println("Error in database selection : ", err)
} else {
	fmt.Println("Selected Dataase ")
}
ctx := driver.WithQueryCount(context.Background())
query := `<some query to return count>`
cursor, err := db.Query(ctx, query, nil)
if err != nil {
	fmt.Println("ERROIR IN INTERLINKING COUNT CURSOR : ", err)
}
var interlinkingCount int
for {

	_, err := cursor.ReadDocument(ctx, &interlinkingCount)
	if driver.IsNoMoreDocuments(err) {
		fmt.Println("NO MORE INTERLINKIGN COUUNT DOCUMENTS : ", err)
		break
	} else if err != nil {
		fmt.Println(" Error in record interlinking count  : ", err)
	}

} /*for*/
defer cursor.Close()

return interlinkingCount

}

There is no way to close the connection object in this function resulting in leak

@ashutosh-akss ashutosh-akss changed the title Memory Leak indriver Memory Leak in go-driver Jun 18, 2017
@ewoutp
Copy link
Contributor

ewoutp commented Jun 19, 2017

Having a close makes a lot of sense.

Why do you make a new connection? You can also re-use the existing connection over and over again.

@ewoutp ewoutp changed the title Memory Leak in go-driver Add active Close method. Jun 19, 2017
@bitstorm-tech
Copy link

Hi all, I stumbled upon the same question. Maybe it would be good to describe what "scope" driver.Client, driver.Database and driver.Collection belong to (if they are long living or short living objects). In my opinion that is not clearly described in the readme.

@GoodVincentTu
Copy link

Hi, any update on this issue? Because I'm using another library which depends on this driver. After doing some code-tracking, I believe if your arango client connection initialized by HTTP, there's no need to close the connection manually.
Since the arango drive instance is a kind of HTTP client wrapper, when the query is executed, the connection will be closed by default. Here's the code:

defer resp.Body.Close()

If no one uses the same HTTP client, Golang utilizes the garbage collection to release the memory.
Therefore, will there be other scenarios that require the explicit Close method?

@GoodVincentTu
Copy link

There is no way to close a connection and it's resulting in memory leak
my 8gb RAM is filled within 1.5 minutes

Get http://localhost:8529/_db/redbus/_api/database/current: dial tcp 127.0.0.1:8529: connect: cannot assign requested address
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x70 pc=0x4bc8be]

I am doing something like this

for _,item := range SomeArray{
ic:=getCount(item)
}

func getCount(Item) (iCount int) {

conn, err := http.NewConnection(http.ConnectionConfig{
	Endpoints: []string{"http://localhost:8529"},
})
if err != nil {
	fmt.Println("Error in Arango Connection", err)
} else {
	// fmt.Println("Arangodb connection successfull")
}
c, err := driver.NewClient(driver.ClientConfig{
	Connection:     conn,
	Authentication: driver.BasicAuthentication("test", "test"),
})

if err != nil {
	fmt.Println("Error in Arango Client Creation", err)
} else {
	fmt.Println("Arangodb client creation successful")
}
db, err := c.Database(nil, "redbus")
if err != nil {
	fmt.Println("Error in database selection : ", err)
} else {
	fmt.Println("Selected Dataase ")
}
ctx := driver.WithQueryCount(context.Background())
query := `<some query to return count>`
cursor, err := db.Query(ctx, query, nil)
if err != nil {
	fmt.Println("ERROIR IN INTERLINKING COUNT CURSOR : ", err)
}
var interlinkingCount int
for {

	_, err := cursor.ReadDocument(ctx, &interlinkingCount)
	if driver.IsNoMoreDocuments(err) {
		fmt.Println("NO MORE INTERLINKIGN COUUNT DOCUMENTS : ", err)
		break
	} else if err != nil {
		fmt.Println(" Error in record interlinking count  : ", err)
	}

} /*for*/
defer cursor.Close()

return interlinkingCount

}

There is no way to close the connection object in this function resulting in leak

Hi, it's a little bit late, however, I just check out your code again. Why did you do the query inside the for-loop instead of constructing your query params outside and batch get your information one time? If there are N objects in your SomeArray array, there'll be O(n) query to your DB requests, and I think that will greatly slow down the execution.

@charlyjna
Copy link

Hi, I am a new user of ArangoDB driver, and I am surprised there is no a such method.
I am writing a program that should be able to close a connection and because there is no Close() method, I do not know how to do.

@antoniodipinto
Copy link

I don't think is needed. Arango works via HTTP REST APIs, so in theory, there is no connection to close. A connection is closed when the request is ended

@jwierzbo
Copy link
Collaborator

In Golang you can close a Connection only on the HTTP response level (resp.Body.Close()) which we already do.
Hence there is no need to expose that on a high level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants