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

Random record and producing duration setting #106

Merged
merged 18 commits into from
Nov 30, 2021

Conversation

chinghongfang
Copy link
Collaborator

@chinghongfang chinghongfang commented Nov 12, 2021

Implement functions in #26.
In this pull request, changes are

  • Add record producing duration.
  • Random record size.
  • Random record content.
  • Change default producing time and records. By default, it runs for about 1 hour. And produce 1,000,000,000 many records.
  • Add unit test of random payload generator.
  • Change consumer completed rate to ratio of number of consumed and number of produced

Add record producing duration.
Random record size.
Random record content.
Change default producing time and records. By default, it
runs for about 68 years (Integer.MAX_VLUE seconds).
long start = System.currentTimeMillis();
producer
.sender()
.topic(param.topic)
.value(payload)
.value(randomPayload.payload())
Copy link
Contributor

Choose a reason for hiding this comment

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

這邊之後要考慮加上key and headers,這樣比較算是完整的record

Copy link
Contributor

Choose a reason for hiding this comment

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

喔 我的意思是之後要發PR去新增key and headers,如果只是填寫null or empty的話那不如先不要

Copy link
Collaborator Author

@chinghongfang chinghongfang Nov 14, 2021

Choose a reason for hiding this comment

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

好的,我下一隻PR把key, header放進去。

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@chinghongfang 感謝更新,幾個小問題請看一下

另外這隻PR除了random data以外還有新增duration,可否更新一下標題包含duration?

app/src/main/java/org/astraea/performance/Performance.java Outdated Show resolved Hide resolved
@chinghongfang chinghongfang changed the title Random record Random record and producing duration setting Nov 12, 2021
By default, produce 10^9 records or produce for 1 hour. Depend on
the first completed condition.
Explicitly set the headers and key to "empty" and "null"
@chia7712
Copy link
Contributor

@chinghongfang 那個百分比是不是要修一下?如果是依照時間來停止的話

@chinghongfang
Copy link
Collaborator Author

@chia7712 好的,現在的做法是雙限制,任一條件到達,就會停止。
可改成輸出百分比較高的(時間%, 數量%),但目前想到可能會發生 consumer完成度 > producer完成度。
我再思考如何解決。

@chinghongfang
Copy link
Collaborator Author

但目前想到可能會發生 consumer完成度 > producer完成度。

想過以後發現,並不會出現 "consumer完成度 > producer完成度" 的情況。

但是有一個問題,依時間來顯示百分比,會不會讓人以為 consumer"完全跟上" producer 的速度?

@chia7712
Copy link
Contributor

依時間來顯示百分比,會不會讓人以為 consumer"完全跟上" producer 的速度?

或許兩者(producer and consumer)應該要用不同邏輯,對producer而言,應該要傳到時間結束為止。而對consumer而言,則應該收到所有這次producers送出去的資料為止

@chinghongfang chinghongfang mentioned this pull request Nov 17, 2021
7 tasks
* Change the meaning of Consumer completion rate.
* Producers produce data until duration end or num of records reach
* Consumers consume data until all data producers produced is consumed.
Copy link
Collaborator Author

@chinghongfang chinghongfang left a comment

Choose a reason for hiding this comment

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

修改輸出內容。

System.out.printf(
"consumer完成度: %.2f%%%n", ((double) result.completedRecords * 100.0 / (double) records));
// Print out percentage of (consumed records) and (produced records)
var percentage = result.completedRecords * 100D / dataManager.produced();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

consumer 完成度改成 "完成和已produce的比例"。

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@chinghongfang 這個版本看起來不錯,幾個小問題麻煩看一下

app/src/main/java/org/astraea/performance/DataManager.java Outdated Show resolved Hide resolved
app/src/main/java/org/astraea/performance/DataManager.java Outdated Show resolved Hide resolved
app/src/main/java/org/astraea/performance/Tracker.java Outdated Show resolved Hide resolved
app/src/main/java/org/astraea/performance/Performance.java Outdated Show resolved Hide resolved
@chinghongfang
Copy link
Collaborator Author

關於 "何時結束" producing/consuming 應該要統一地方做判斷,不把判斷分散各地,可以使程式更清楚明瞭。
目前打算把判斷放在DataManager內。

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@chinghongfang 感謝更新,一些設計建議請看一下,謝謝

app/src/main/java/org/astraea/performance/Manager.java Outdated Show resolved Hide resolved
app/src/main/java/org/astraea/performance/Manager.java Outdated Show resolved Hide resolved
app/src/main/java/org/astraea/performance/Manager.java Outdated Show resolved Hide resolved
app/src/main/java/org/astraea/performance/Manager.java Outdated Show resolved Hide resolved
app/src/main/java/org/astraea/performance/Manager.java Outdated Show resolved Hide resolved
app/src/main/java/org/astraea/performance/Manager.java Outdated Show resolved Hide resolved
app/src/main/java/org/astraea/performance/Performance.java Outdated Show resolved Hide resolved
@chinghongfang
Copy link
Collaborator Author

這次主要更新

  • 記錄producer是否完成
  • 更改consumer thread停止條件
  • 更改API,改成指定"records數量"或"持續時間"
  • 避免重複的運算(metrics, manager重複紀錄record數量)
  • Metrics 實作Biconsumer<>

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@chinghongfang 幾個小建議請看一下

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@chinghongfang 感謝更新,還有幾個意見麻煩看一下

Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@chinghongfang 我用這隻PR測試了一下100000records,然後用我們的offset工具來看,資料筆數會不一樣,可否麻煩你檢查一下

System.out.printf(
"consumer完成度: %.2f%%%n", ((double) result.completedRecords * 100.0 / (double) records));
// Print out percentage of (consumed records) and (produced records)
var percentage = result.completedRecords * 100D / manager.producedRecords();
Copy link
Contributor

Choose a reason for hiding this comment

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

這個算法是不是怪怪的?producedRecords是會隨時間增長的值,拿它來算比例怪怪的

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

因為無法確切的知道總共會有幾筆資料要被產出(當我們指定執行時間時),所以這裡改成了和目前生產量的比例。

還是要改回之前的比例;而當我們指定執行時間時,就不輸出完成度?

Copy link
Contributor

Choose a reason for hiding this comment

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

有點忘了之前的討論結果,在時間模式下已經有時間的輸出了?如果是的話,或許這邊就不用再輸出比例了,不然很容易誤會

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

時間模式的話,會持續consume 直到所有的資料都收到。

Copy link
Contributor

Choose a reason for hiding this comment

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

會持續consume 直到所有的資料都收到。

這個是指持續到producers都完成,且所有送出去的資料都收到吧?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

這個是指持續到producers都完成,且所有送出去的資料都收到吧?

是的。

Fix `CountDownLatch` misuse.
@chia7712 chia7712 merged commit a38072b into opensource4you:main Nov 30, 2021
@chia7712
Copy link
Contributor

@chinghongfang 感謝這次的改良!

@chinghongfang chinghongfang deleted the randomRecord branch March 2, 2022 09:01
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.

2 participants