confluo icon indicating copy to clipboard operation
confluo copied to clipboard

Published 20 hours ago • verified publisher icon ucbrise

Java pub-sub Implementation

Open rudy2steiner opened this issue 6 years ago • 16 comments

What changes were proposed in this pull request?

  1. provide a producer and consumer implement for confluo as a pub/sub system in java
  2. also include a simple produce/consume benchmark test

How was this patch tested?

TestPubSub class provide unit tests

related issue #123 ,#125

rudy2steiner avatar Jan 27 '19 16:01 rudy2steiner

Can one of the admins verify this patch?

AmplabJenkins avatar Jan 27 '19 16:01 AmplabJenkins

Jenkins test this please.

anuragkh avatar Jan 27 '19 20:01 anuragkh

Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/confluo-prb/360/ Test FAILed.

AmplabJenkins avatar Jan 27 '19 20:01 AmplabJenkins

Jenkins test this please.

rudy2steiner avatar Jan 28 '19 19:01 rudy2steiner

Jenkins test this please.

anuragkh avatar Jan 28 '19 19:01 anuragkh

Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/confluo-prb/361/ Test PASSed.

AmplabJenkins avatar Jan 28 '19 19:01 AmplabJenkins

thanks for your code review, i'll try to optimize my implementation following your advices in next few days @anuragkh

rudy2steiner avatar Jan 29 '19 03:01 rudy2steiner

Thanks for submitting the PR! Look forward to your changes.

anuragkh avatar Jan 29 '19 03:01 anuragkh

@anuragkh you may review this PR again

rudy2steiner avatar Feb 13 '19 16:02 rudy2steiner

Most of the changes look great! I left some minor comments on formatting, it will make the code much more readable if we can get those in.

anuragkh avatar Feb 19 '19 10:02 anuragkh

add ConfluoProducer/Cosumer pub-sub test,hope my changes make my PR more readable

rudy2steiner avatar Feb 21 '19 16:02 rudy2steiner

Hi @rudy2steiner, sorry I was traveling for the past few days. I think this patch still needs better tests -- please see the relevant comments on the test.

anuragkh avatar Mar 07 '19 04:03 anuragkh

removed non-unit test

rudy2steiner avatar Mar 08 '19 09:03 rudy2steiner

Hi @rudy2steiner, thanks for that change. What I meant was we need to add tests that assert the correctness of the implementation. Currently the existing tests do not assert anything. We should update the tests to make sure we are checking for correctness using asserts.

anuragkh avatar Mar 12 '19 16:03 anuragkh

Have you notice the major change in the class TestPubSub in the commit: https://github.com/ucbrise/confluo/pull/135/commits/7f9e0cbd41f625a9b37e6b0b7df035ccc74627f9; And the newest TestPubSub.java contains the testConfluoPubSub function, in that i have 4 steps to assert the correctness of the implements(produce/consume) as following:

  • record the current offset,named mq_offset_mark, in target topic,eg mq_001
  • produce some of messages(pMeesages) to mq_001,
  • starting consume from the old offset mq_offset_mark
  • assert consume messages(cMessages) with pMessages in order

https://github.com/ucbrise/confluo/blob/7f9e0cbd41f625a9b37e6b0b7df035ccc74627f9/javaclient/src/test/java/TestPubSub.java

rudy2steiner avatar Mar 14 '19 02:03 rudy2steiner

Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/confluo-prb/387/

AmplabJenkins avatar Mar 16 '21 03:03 AmplabJenkins