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

🙋 LDAP 初始化连接问题 #50

Closed
ischenxin opened this issue Jul 4, 2022 · 6 comments · Fixed by #95
Closed

🙋 LDAP 初始化连接问题 #50

ischenxin opened this issue Jul 4, 2022 · 6 comments · Fixed by #95
Labels
question Further information is requested

Comments

@ischenxin
Copy link

请问作者 ,下面这种写法不会导致 连接超时/丢失 的问题吗

public/common/ldap.go

	err = ldap.Bind(config.Conf.Ldap.AdminDN, config.Conf.Ldap.AdminPass)
	if err != nil {
		Log.Panicf("绑定admin账号异常: %v", err)
		panic(fmt.Errorf("绑定admin账号异常: %v", err))
	}

	// 全局LDAP赋值
	LDAP = ldap
@ischenxin ischenxin added the question Further information is requested label Jul 4, 2022
@eryajf
Copy link
Member

eryajf commented Jul 4, 2022

您好 @ischenxin👋,我已收到您的反馈,我将安排时间考虑您提交的信息并进行回复。-- 这条信息是由自动回复的机器人发出的。

Hello @ischenxin. I have received your feedback, and I will arrange time to consider the information you submitted and reply. -- This message is sent by an automatic reply robot.

@eryajf
Copy link
Member

eryajf commented Jul 4, 2022

这是个好问题。
这里是项目启动的时候会与ldap建连,如果连接有问题,则项目启动会失败。
不过也确实有一些问题,比如如果ldap重启之后,这个连接就会丢失了。如果改成每次请求都建立连接,我有点担心会不会加大请求失败的几率,这块儿的确需要思索一下。

@ischenxin
Copy link
Author

这是个好问题。 这里是项目启动的时候会与ldap建连,如果连接有问题,则项目启动会失败。 不过也确实有一些问题,比如如果ldap重启之后,这个连接就会丢失了。如果改成每次请求都建立连接,我有点担心会不会加大请求失败的几率,这块儿的确需要思索一下。

是的,因为这个只是一个连接,假如ldap配置了连接最大时间 或者连接因为网络问题断开,那么后续的请求应该都会报错

@ischenxin
Copy link
Author

这里可以考虑每次查询的时候 检查一下当前连接是否close ,如果 close ,重连一下即可

类似

if s.ldap.IsClosing() {
    s.ldap.Start()
}

@eryajf
Copy link
Member

eryajf commented Jul 4, 2022

@ischenxin 如果方便的话,你可以基于此问题走个PR?

@eryajf
Copy link
Member

eryajf commented Jul 5, 2022

我尝试对此进行处理,感觉不太理想,几天前有人在官方提到过这个问题: go-ldap/ldap#383

我对连接池的概念原理还不甚了解,所以等官方给出理想的方案之后,我再针对这个问题进行处理

@eryajf eryajf linked a pull request Jul 24, 2022 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants