Skip to content

Use ImmutableWorkerInfo instead of ZKWorker#2588

Merged
drcrallen merged 1 commit intoapache:masterfrom
metamx:abstract-zkworker
Mar 15, 2016
Merged

Use ImmutableWorkerInfo instead of ZKWorker#2588
drcrallen merged 1 commit intoapache:masterfrom
metamx:abstract-zkworker

Conversation

@nishantmonu51
Copy link
Copy Markdown
Member

This PR is follow up changes for #2249 and #2585
#2249 uses Worker instead of ZkWorker whenever possible for task tiering.

This PR changes the behavior to expose ImmutableWorkerInfo from WorkerTaskRunner instead of Worker. ImmutableWorkerInfo has more details like list of running tasks and availability groups.

These extra details exposed are required for #2086 (implementing pending task based resource management strategy)

@nishantmonu51
Copy link
Copy Markdown
Member Author

@gianm @drcrallen: Since its mostly related to your changes, please have a look at it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

... worker

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Incomplete javadoc :(

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Mar 12, 2016

@nishantmonu51 looks good other than some minor comments, let me know when you are ready and I can take another look

@nishantmonu51 nishantmonu51 force-pushed the abstract-zkworker branch 2 times, most recently from b7d1896 to 87203d8 Compare March 14, 2016 18:15
@nishantmonu51
Copy link
Copy Markdown
Member Author

@drcrallen @gianm handed review comments, please check again.

review comments

add test for equals and hashcode
@gianm
Copy link
Copy Markdown
Contributor

gianm commented Mar 14, 2016

👍 after travis

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Mar 14, 2016

Tests run: 5, Failures: 0, Errors: 2, Skipped: 0, Time elapsed: 34.79 sec <<< FAILURE! - in io.druid.indexing.worker.WorkerTaskMonitorTest
testRestartCleansOldStatus(io.druid.indexing.worker.WorkerTaskMonitorTest) Time elapsed: 31.087 sec <<< ERROR!
java.lang.Exception: test timed out after 30000 milliseconds
at sun.misc.Unsafe.park(Native Method)
at java.util.concurrent.locks.LockSupport.park(LockSupport.java:186)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:834)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireSharedInterruptibly(AbstractQueuedSynchronizer.java:994)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1303)
at java.util.concurrent.CountDownLatch.await(CountDownLatch.java:236)
at io.druid.indexing.worker.WorkerTaskMonitor.stop(WorkerTaskMonitor.java:260)
at io.druid.indexing.worker.WorkerTaskMonitorTest.testRestartCleansOldStatus(WorkerTaskMonitorTest.java:331)
testRestartCleansOldStatus(io.druid.indexing.worker.WorkerTaskMonitorTest) Time elapsed: 31.087 sec <<< ERROR!
java.lang.IllegalStateException: not started
at com.google.common.base.Preconditions.checkState(Preconditions.java:176)
at io.druid.indexing.worker.WorkerTaskMonitor.stop(WorkerTaskMonitor.java:250)
at io.druid.indexing.worker.WorkerTaskMonitorTest.tearDown(WorkerTaskMonitorTest.java:205)
Running io.druid.indexing.worker.http.WorkerResourceTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.809 sec - in io.druid.indexing.worker.http.WorkerResourceTest
Running io.druid.indexing.worker.TaskAnnouncementTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.013 sec - in io.druid.indexing.worker.TaskAnnouncementTest
Running io.druid.indexing.appenderator.ActionBasedUsedSegmentCheckerTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 sec - in io.druid.indexing.appenderator.ActionBasedUsedSegmentCheckerTest
Running io.druid.server.initialization.IndexerZkConfigTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.874 sec - in io.druid.server.initialization.IndexerZkConfigTest
Results :
Tests in error:
io.druid.indexing.worker.WorkerTaskMonitorTest.testRestartCleansOldStatus(io.druid.indexing.worker.WorkerTaskMonitorTest)
Run 1: WorkerTaskMonitorTest.testRestartCleansOldStatus:331 » test timed out after 3...
Run 2: WorkerTaskMonitorTest.tearDown:205 » IllegalState not started

@fjy fjy closed this Mar 14, 2016
@fjy fjy reopened this Mar 14, 2016
* Once a task completes, it is up to the RTR to remove the task status and run any necessary cleanup.
* The RemoteTaskRunner is event driven and updates state according to ephemeral node changes in ZK.
* <p/>
* <p>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

did these doc changes need to happen?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

its done by intellij with the code formatting. so probably its fine to go along.

@drcrallen
Copy link
Copy Markdown
Contributor

👍 (after travis) minor question about javadoc at author's discretion.

@drcrallen drcrallen removed their assignment Mar 14, 2016
drcrallen added a commit that referenced this pull request Mar 15, 2016
Use ImmutableWorkerInfo instead of ZKWorker
@drcrallen drcrallen merged commit 32399bc into apache:master Mar 15, 2016
@drcrallen drcrallen deleted the abstract-zkworker branch March 15, 2016 17:27
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