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

CDAP-13380 CDAP Cloud Logging Integration #10334

Merged
merged 5 commits into from Jul 22, 2018

Conversation

@anwar6953
Copy link
Contributor

commented Jul 13, 2018

The first commit is a revert of a revert: #10305, where the original PR was: #10258.

I reverted it because it had a bug, which presented itself as high CPU usage, and everything being much slower. For instance, programs took about twice as long to launch and run.
The second commit fixes the bug.

The third commit implements the functionality for distributed CDAP.

@anwar6953 anwar6953 force-pushed the bugfix_release/CDAP-13380 branch 3 times, most recently from 866a9d3 to a40ff81 Jul 13, 2018

@Override
public void process(Iterator<byte[]> loggingEventBytes) {
// TODO (CDAP-13380): implement distributed log processor

This comment has been minimized.

Copy link
@chtyim

chtyim Jul 15, 2018

Contributor

Still a TODO??

This comment has been minimized.

Copy link
@anwar6953

anwar6953 Jul 16, 2018

Author Contributor

Ah, the implementation doesn't use this class. I will remove this class, as it is unused.

@anwar6953 anwar6953 force-pushed the bugfix_release/CDAP-13380 branch from aad6e36 to c296f00 Jul 16, 2018
@prinam prinam added the 5.0 label Jul 16, 2018
… interrupted... see CDAP-8041 for a similar scenario).
@anwar6953

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2018

Copy link
Contributor

left a comment

Just one comment about a misplaced comment. rest LGTM.

buffer.clear();
failures = 0;

// Log using the status manager

This comment has been minimized.

Copy link
@chtyim

chtyim Jul 19, 2018

Contributor

This comment is not right.

@anwar6953 anwar6953 force-pushed the bugfix_release/CDAP-13380 branch from 4aec5ab to e54cf72 Jul 19, 2018
@anwar6953

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2018

Addressed the comment, RUT build is green.
However, ITN is not passing. I'm currently investigating.

@anwar6953 anwar6953 force-pushed the bugfix_release/CDAP-13380 branch from e54cf72 to a08d9be Jul 20, 2018
@anwar6953

This comment has been minimized.

Copy link
Contributor Author

commented Jul 20, 2018

@anwar6953

This comment has been minimized.

Copy link
Contributor Author

commented Jul 21, 2018

Removed the latest refactoring commit as it was causing ITN failures. ITN passes now.
The build is also failing on flaky tests: https://builds.cask.co/browse/CDAP-RUT1538-10

@anwar6953 anwar6953 merged commit 64e1ab7 into release/5.0 Jul 22, 2018
@anwar6953 anwar6953 deleted the bugfix_release/CDAP-13380 branch Jul 22, 2018
anwar6953 pushed a commit that referenced this pull request Jul 22, 2018
@anwar6953 anwar6953 referenced this pull request Jul 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.