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

[AMBARI-23793]. MySQL Connector JAR distribution is broken (amagyar) #1219

Merged
merged 4 commits into from May 11, 2018

Conversation

zeroflag
Copy link
Contributor

@zeroflag zeroflag commented May 9, 2018

What changes were proposed in this pull request?

After running ambari-server setup --jdbc-db=mysql ambari.properties file changes, but these changes are not visible in ambariLevelParams of command jsons. Amabari agent still sees a cached version of these parameters.

This fix triggers an event which invalidates the cache when ambari.properties file changes.

How was this patch tested?

I ran the following command

$ ambari-server setup --jdbc-db=mysql --jdbc-driver=/usr/share/java/mysql-connector-java.jar

and inspected ambariLevel params of /var/lib/ambari-agent/data/command-XXX.json

@asfgit
Copy link

asfgit commented May 9, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/2195/
Test FAILed.
Test FAILured.

@asfgit
Copy link

asfgit commented May 9, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/2197/
Test FAILed.
Test FAILured.

@zeroflag
Copy link
Contributor Author

zeroflag commented May 9, 2018

retest this please

@asfgit
Copy link

asfgit commented May 9, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/2198/
Test FAILed.
Test FAILured.

import java.io.File;
import java.io.IOException;

import org.apache.commons.lang3.SystemUtils;
Copy link
Member

Choose a reason for hiding this comment

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

Wrong import (lang3)?


package org.apache.ambari.server.events;

public class AmbariPropertiesChangedEvent extends AmbariEvent {
Copy link
Member

Choose a reason for hiding this comment

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

JavaDoc

}

private void notifyListener(Path path) {
LOG.info(path + " changed. Sending notification.");
Copy link
Member

Choose a reason for hiding this comment

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

Use {} for log statements

/**
* Watchdog that notifies a listener on a file content change.
*/
public class SingleFileWatch implements Runnable {
Copy link
Member

Choose a reason for hiding this comment

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

Since the WatchService can only watch directories, should this class instead spawn a single thread per directory (not file)?

Copy link
Contributor Author

@zeroflag zeroflag May 9, 2018

Choose a reason for hiding this comment

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

I don't think we need that functionality at this moment. This is intended to watch a single file, but it's implemented in way to watch the parent directory of that file because of the limitations of WatchService. From usability standpoint this is simpler than watching a directory and doing the filtering outside or registering multiple listeners per each file.

@asfgit
Copy link

asfgit commented May 9, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/2199/
Test FAILed.
Test FAILured.

@ncole
Copy link
Contributor

ncole commented May 9, 2018

Please check the feasibility of using commons FileAlterationListener and the like. Also please do some basic profiling (on any implementation) to make sure CPU utilization remains low. We don't want 100% cycles checking on ambari.properties.

@zeroflag
Copy link
Contributor Author

zeroflag commented May 9, 2018

FileAlterationMonitor, FileWatchDog and other 3rd party watchers use periodic polling to check file changes. WatchService uses the operating system specific event driven primitives if available and falls backs to polling if it's not available (for example on OSX). I'll check the CPU usage.

@adoroszlai
Copy link
Contributor

retest this please

@asfgit
Copy link

asfgit commented May 9, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/2205/
Test FAILed.
Test FAILured.

@zeroflag
Copy link
Contributor Author

I discovered some problems with WatchService (duplicated events, works slightly differently on some platforms), so a replaced it with an apache watchdog. Please review again.

@asfgit
Copy link

asfgit commented May 10, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/2210/
Test PASSed.

@zeroflag zeroflag merged commit ad366eb into apache:trunk May 11, 2018
@zeroflag zeroflag deleted the AMBARI-23793-trunk branch May 11, 2018 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants