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

feat: support passive auto discovery #267

Merged
merged 3 commits into from
Sep 6, 2024
Merged

Conversation

ZhiHanZ
Copy link
Collaborator

@ZhiHanZ ZhiHanZ commented Sep 5, 2024

No description provided.

*/
void tryAutoDiscovery(OkHttpClient client, ClientSettings settings) {
if (this.autoDiscovery) {
if (this.driverUri.enableMock()) {
Copy link
Member

Choose a reason for hiding this comment

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

neat mock 👍

logger.log(Level.WARNING, "Current Query Node do not support auto discovery, close the functionality: " + e.getMessage());
this.autoDiscovery = false;
} catch (Exception e) {
logger.log(Level.FINE, "Error auto discovery: " + " cause: " + e.getCause() + " message: " + e.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

first time to find out FINE log level, what's it's normal use case?

(not an issue at all, just curious 👀)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

info level will print problem by default, I think it is similar to DEBUG level on golang

Copy link
Member

@hantmac hantmac left a comment

Choose a reason for hiding this comment

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

LGTM.

@ZhiHanZ ZhiHanZ requested a review from flaneur2020 September 6, 2024 02:50

public class DatabendNodes implements DatabendNodeRouter {

private AtomicReference<List<URI>> query_nodes_uris;
protected final AtomicInteger index;
// keep track of latest discovery scheduled time
protected final AtomicReference<Long> lastDiscoveryTime = new AtomicReference<>(0L);
private static final Logger logger = Logger.getLogger(DatabendNodes.class.getPackage().getName());
@Setter
private boolean debug = false;
Copy link
Member

Choose a reason for hiding this comment

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

i guess if there is only one setter, define a setDebug is preferrable than using lomok

Copy link
Member

@flaneur2020 flaneur2020 left a comment

Choose a reason for hiding this comment

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

LGTM 👍, only one small style issue about lomok, it's not important

@ZhiHanZ ZhiHanZ merged commit 19245ea into databendlabs:main Sep 6, 2024
2 checks passed
cdmikechen pushed a commit to cdmikechen/databend-jdbc that referenced this pull request Oct 29, 2024
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

Successfully merging this pull request may close these issues.

3 participants