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

Improve embedded package documentation #2326

Closed
egplat opened this issue Mar 30, 2023 · 6 comments · Fixed by #3537
Closed

Improve embedded package documentation #2326

egplat opened this issue Mar 30, 2023 · 6 comments · Fixed by #3537
Assignees
Labels
code/chore Code maintenance improvements community Issues and PRs assigned to community members good first issue Good issues for new external contributors
Milestone

Comments

@egplat
Copy link

egplat commented Mar 30, 2023

We should update code comments. See #2326 (comment).


Versions

0.9.3

What did you do?

f, err := ferretdb.New(&ferretdb.Config{
		Listener: ferretdb.ListenerConfig{
			TCP: "127.0.0.1:17027",
		},
		Handler:       "pg",
		PostgreSQLURL: "postgres://127.0.0.1:5432/ferretdb", // ferretdb 对齐PostgreSQL的数据库
	})
	if err != nil {
		log.Fatal(err)
	}

ctx, cancel := context.WithCancel(context.Background())
uri := f.MongoDBURI()
fmt.Println(uri)

I created an instance of FerretDB using the code above, but did not run FerretDB.Run(). Then I tried to get the MongoDB uri, which caused the program to block at <-l.tcpListenerReady and ultimately resulted in a deadlock.

What did you expect to see?

Not running FerretDB.Run() and then obtaining the MongoDB uri will not lead to a deadlock.

What did you see instead?

fatal error: all goroutines are asleep - deadlock!

goroutine 1 [chan receive]:
https://github.com/FerretDB/FerretDB/internal/clientconn.(*Listener).TCPAddr(...)
/internal/clientconn/listener.go:321
https://github.com/FerretDB/FerretDB/ferretdb.(*FerretDB).MongoDBURI(0x1057920d8?)
/ferretdb/ferretdb.go:186 +0xdc

Environment

- OS:
- Architecture:
- Version:
- Deployment:
- Deployment details:
@egplat egplat added the code/bug Some user-visible feature works incorrectly label Mar 30, 2023
@AlekSi
Copy link
Member

AlekSi commented Apr 5, 2023

Unfortunately, starting embedded FerretDB with a Run method is required. For example, when the TCP address is 127.0.0.1:0, we should first begin listening on a TCP socket to know the allocated port number. And we don't want to make Run sometimes required and sometimes not.

But I think we could explain it better in code comments / documentation. Could you send a PR?

@AlekSi AlekSi added code/chore Code maintenance improvements good first issue Good issues for new external contributors and removed code/bug Some user-visible feature works incorrectly labels Apr 5, 2023
@egplat
Copy link
Author

egplat commented Apr 5, 2023 via email

@AlekSi AlekSi changed the title get TCP/UNIX/TLS listener's address may cause "goroutines are asleep - deadlock" Improve embedded package documentation Apr 5, 2023
@princejha95
Copy link
Contributor

Will pick this up.

@egplat
Copy link
Author

egplat commented Oct 5, 2023 via email

@princejha95
Copy link
Contributor

princejha95 commented Oct 5, 2023

Unfortunately, starting embedded FerretDB with a Run method is required. For example, when the TCP address is 127.0.0.1:0, we should first begin listening on a TCP socket to know the allocated port number. And we don't want to make Run sometimes required and sometimes not.

But I think we could explain it better in code comments / documentation. Could you send a PR?

On top of what @AlekSi mentioned, mongoDB URI is formed using the listener's address and in order to get that, listener has to be instantiated which is done by FerretDB.Run() method which implies the necessity of running it before running FerretDB.MongoDBURI() method.

Please correct me if my understanding is wrong.

Thanks

@AlekSi
Copy link
Member

AlekSi commented Oct 6, 2023

That's correct

@AlekSi AlekSi added the community Issues and PRs assigned to community members label Oct 9, 2023
@AlekSi AlekSi added this to the Next milestone Oct 10, 2023
@AlekSi AlekSi self-assigned this Oct 10, 2023
AlekSi pushed a commit that referenced this issue Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/chore Code maintenance improvements community Issues and PRs assigned to community members good first issue Good issues for new external contributors
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants