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

RYA-443 Rya Streams Query Manager daemon program. #272

Closed

Conversation

kchilton2
Copy link
Contributor

Description

Implemented an RPM installed CentOS 7 daemon service used to execute Kafka Streams jobs on the local machine. This is part of Rya Streams.

Tests

Wrote automated tests and executed the application on a CentOS 7 machine using the RPM.

Links

https://issues.apache.org/jira/browse/RYA-443

@kchilton2 kchilton2 force-pushed the RYA-443_rya-streams-querymanager branch from b889bc0 to 50a8121 Compare February 8, 2018 21:30
@asfgit
Copy link

asfgit commented Feb 8, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/686/

Build result: FAILURE

[...truncated 3.04 MB...][INFO] Apache Rya Web Projects ............................ SKIPPED[INFO] Apache Rya Web Implementation ...................... SKIPPED[INFO] ------------------------------------------------------------------------[INFO] BUILD FAILURE[INFO] ------------------------------------------------------------------------[INFO] Total time: 38:10 min[INFO] Finished at: 2018-02-08T22:18:44+00:00[INFO] Final Memory: 301M/2452M[INFO] ------------------------------------------------------------------------Waiting for Jenkins to finish collecting data[ERROR] Failed to execute goal org.codehaus.mojo:rpm-maven-plugin:2.1.5:attached-rpm (create-rpm-distribution) on project rya.streams.query-manager: Unable to query for default vendor from RPM: Error while executing process. Cannot run program "rpm": error=2, No such file or directory -> [Help 1][ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.[ERROR] Re-run Maven using the -X switch to enable full debug logging.[ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles:[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :rya.streams.query-managerchannel stoppedSetting status of b889bc0 to FAILURE with url https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/686/ and message: 'FAILURE 'Using context: Jenkins: clean package -Pgeoindexing

@asfgit
Copy link

asfgit commented Feb 8, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/687/

Build result: FAILURE

[...truncated 3.01 MB...][INFO] Apache Rya Spark Support ........................... SKIPPED[INFO] Apache Rya Web Projects ............................ SKIPPED[INFO] Apache Rya Web Implementation ...................... SKIPPED[INFO] ------------------------------------------------------------------------[INFO] BUILD FAILURE[INFO] ------------------------------------------------------------------------[INFO] Total time: 40:57 min[INFO] Finished at: 2018-02-08T22:25:43+00:00[INFO] Final Memory: 546M/3106M[INFO] ------------------------------------------------------------------------[ERROR] Failed to execute goal org.codehaus.mojo:rpm-maven-plugin:2.1.5:attached-rpm (create-rpm-distribution) on project rya.streams.query-manager: Unable to query for default vendor from RPM: Error while executing process. Cannot run program "rpm": error=2, No such file or directory -> [Help 1][ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.[ERROR] Re-run Maven using the -X switch to enable full debug logging.[ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles:[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :rya.streams.query-managerchannel stoppedSetting status of 50a8121 to FAILURE with url https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/687/ and message: 'FAILURE 'Using context: Jenkins: clean package -Pgeoindexing

Copy link
Contributor

@ejwhite922 ejwhite922 left a comment

Choose a reason for hiding this comment

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

Looks good. Mostly minor stuff.

sb.append(getSparql() + "\n");
sb.append("Is ");
if (!isActive) {
sb.append(" Not ");
Copy link
Contributor

Choose a reason for hiding this comment

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

No space is needed before "Not". The previous append has one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

} finally {
lock.unlock();
}
log.trace("runOneIteration() - Exit");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this log message be moved up into the finally block so it's called if an Exception is thrown?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, done.

" Query ID: " + queryId + ",\n" +
" Change Type: " + changeType + ",\n" +
" Is Active: " + isActive + ",\n" +
" SPARQL: " + sparql + "\n" +
Copy link
Contributor

Choose a reason for hiding this comment

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

Is "sparql" coming from user input? Might want to wrap that in org.apache.rya.api.log.LogUtils.clean(sparql)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What does that function do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to assume it is sanitized by the time it reaches this line of code.

// Listing the queries should work using an initialized change log.
final Set<StreamsQuery> stored = initializedQueries.list();
assertEquals(expected, stored);
} finally {
queries.stop();
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems redundant with the outer try-finally block calling stop(). Unless the intent is to test it handling 2 consecutive calls to stop(). The inner try-finally can probably be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

}

/**
* Get the Rya instance name from a Kafka topic name that has been used for a {@link QueryChnageLog}.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo. QueryChangeLog

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

/**
* Represents a Kafka Server that manages {@link KafkaQueryChangeLog}s.
* <p/>
* Thread safe.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to @ThreadSafe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the dependency for that annotation?

Copy link
Contributor

Choose a reason for hiding this comment

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

net.jcip.annotations.ThreadSafe

Looks like Rya is using this one:

        <dependency>
            <groupId>com.github.stephenc.jcip</groupId>
            <artifactId>jcip-annotations</artifactId>
            <version>1.0-1</version>
        </dependency>

/**
* Utilities that are useful for interacting with {@link Thread}s while testing.
*/
public class ThreadUtil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a private constructor to prevent instantiation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

sb.append("ID: ").append(query.getQueryId())
.append(" ")
.append("Is Active: ").append(query.isActive())
.append( query.isActive() ? " " : " " )
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe change to:

.append("Is Active: ")

.append(StringUtils.rightPad(query.isActive(), 9))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

sb.append("ID: ");
sb.append(getQueryId().toString() + "\n");
sb.append("Query: ");
sb.append(getSparql() + "\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is "getSparql()" coming from user input? Might want to wrap that in org.apache.rya.api.log.LogUtils.clean(getSparql())

Copy link
Contributor Author

@kchilton2 kchilton2 Feb 14, 2018

Choose a reason for hiding this comment

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

Yes, but it should be sanitized before it gets here.

.append(" ")
.append("Is Active: ").append(query.isActive())
.append( query.isActive() ? " " : " " )
.append("Query: ").append(query.getSparql()).append("\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

LogUtils.clean(query.getSparql())

Copy link
Contributor Author

@kchilton2 kchilton2 Feb 14, 2018

Choose a reason for hiding this comment

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

Should be sanitized before it gets here.

@kchilton2
Copy link
Contributor Author

How do we want to deal with this project producing an RPM? The ASF jenkins doesn't have the rpm command installed.

@asfgit
Copy link

asfgit commented Feb 14, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/691/

Build result: FAILURE

[...truncated 3.03 MB...][INFO] Apache Rya Spark Support ........................... SKIPPED[INFO] Apache Rya Web Projects ............................ SKIPPED[INFO] Apache Rya Web Implementation ...................... SKIPPED[INFO] ------------------------------------------------------------------------[INFO] BUILD FAILURE[INFO] ------------------------------------------------------------------------[INFO] Total time: 37:36 min[INFO] Finished at: 2018-02-14T20:56:07+00:00[INFO] Final Memory: 535M/2904M[INFO] ------------------------------------------------------------------------[ERROR] Failed to execute goal org.codehaus.mojo:rpm-maven-plugin:2.1.5:attached-rpm (create-rpm-distribution) on project rya.streams.query-manager: Unable to query for default vendor from RPM: Error while executing process. Cannot run program "rpm": error=2, No such file or directory -> [Help 1][ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.[ERROR] Re-run Maven using the -X switch to enable full debug logging.[ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles:[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :rya.streams.query-managerchannel stoppedSetting status of 6428ca8 to FAILURE with url https://builds.apache.org/job/incubator-rya-master-with-optionals-pull-requests/691/ and message: 'FAILURE 'Using context: Jenkins: clean package -Pgeoindexing

Copy link
Contributor

@ejwhite922 ejwhite922 left a comment

Choose a reason for hiding this comment

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

Looks good.

@kchilton2
Copy link
Contributor Author

I'm closing this because #279 takes it over.

@kchilton2 kchilton2 closed this Mar 2, 2018
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.

4 participants