You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/03/05 19:32:55 UTC
[GitHub] [incubator-pinot] mqliang opened a new pull request #6650: complete compatibility regression testing
mqliang opened a new pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650
## Description
* split compCheck.sh as two script so we can build and test separately
* redirect services' logs to a file for easy of debug
* complete and enable test during rolling upgrade/rollback
* fix some minor bugs.
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR otherwise need attention when creating release notes? Things to consider:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins removed
* [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
## Release Notes
If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release.
If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text
## Documentation
If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mqliang commented on a change in pull request #6650: complete compatibility regression testing
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650#discussion_r591767270
##########
File path: compatibility-verifier/compCheck.sh
##########
@@ -105,13 +102,18 @@ function stopService() {
else
echo "Pid file ${dirName}/${serviceName}.pid not found. Failed to stop component ${serviceName}"
fi
+ echo "${serviceName} stopped"
}
# Starts a Pinot cluster given a specific target directory
function startServices() {
dirName=$1
startService zookeeper "$dirName"
+ # Controller depends on zookeeper, if not wait zookeeper to be ready, controller will crash.
+ waitForZkReady
startService controller "$dirName"
+ # Broker depends on controller, if not wait controller to be ready, broker will crash.
+ waitForControllerReady
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6650: complete compatibility regression testing
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650#discussion_r588800523
##########
File path: compatibility-verifier/compCheck.sh
##########
@@ -34,61 +34,45 @@
# and run all the scripts in the directory in alpha order, one script at each
# "stage" of upgrade.
#
-# We may modify to choose a minimal run in which the same set of operatons are run
+# We may modify to choose a minimal run in which the same set of operations are run
# between any two component upgrades/rollbacks -- this may consist of adding
# one more segment to table, adding some more rows to the stream topic, and
# running some queries with the new data.
-# get a temporary directory in case the workingDir is not provided by user
-TMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'mytmpdir')
-
-COMPAT_TESTER_PATH="pinot-integration-tests/target/pinot-integration-tests-pkg/bin/pinot-compat-test-runner.sh"
# get usage of the script
function usage() {
command=$1
- echo "Usage: $command olderCommit newerCommit [workingDir]"
+ echo "Usage: $command [workingDir]"
exit 1
}
-# cleanup the temporary directory when exiting the script
-function cleanup() {
- if [ -n "$TMP_DIR" ] && [ -d "$TMP_DIR" ] && [ "$workingDir" = "$TMP_DIR" ] ; then
- echo "The temporary directory $TMP_DIR needs to be cleaned up."
- fi
-}
-
-# This function builds Pinot given a specific commit hash and target directory
-function checkoutAndBuild() {
- commitHash=$1
- targetDir=$2
-
- pushd "$targetDir" || exit 1
- git init
- git remote add origin https://github.com/apache/incubator-pinot
- git fetch --depth 1 origin "$commitHash"
- git checkout FETCH_HEAD
- mvn install package -DskipTests -Pbin-dist
- popd || exit 1
-}
-
# Given a component and directory, start that version of the specific component
function startService() {
serviceName=$1
dirName=$2
# Upon start, save the pid of the process for a component into a file in /tmp/{component}.pid, which is then used to stop it
pushd "$dirName"/pinot-tools/target/pinot-tools-pkg/bin || exit 1
if [ "$serviceName" = "zookeeper" ]; then
- sh -c 'echo $$ > $0/zookeeper.pid; exec ./pinot-admin.sh StartZookeeper' "${dirName}" &
+ sh -c 'rm -rf /tmp/zkdir'
Review comment:
Instead of `/tmp` use the working dir here
##########
File path: compatibility-verifier/compCheck.sh
##########
@@ -34,61 +34,45 @@
# and run all the scripts in the directory in alpha order, one script at each
# "stage" of upgrade.
#
-# We may modify to choose a minimal run in which the same set of operatons are run
+# We may modify to choose a minimal run in which the same set of operations are run
# between any two component upgrades/rollbacks -- this may consist of adding
# one more segment to table, adding some more rows to the stream topic, and
# running some queries with the new data.
-# get a temporary directory in case the workingDir is not provided by user
-TMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'mytmpdir')
-
-COMPAT_TESTER_PATH="pinot-integration-tests/target/pinot-integration-tests-pkg/bin/pinot-compat-test-runner.sh"
# get usage of the script
function usage() {
command=$1
- echo "Usage: $command olderCommit newerCommit [workingDir]"
+ echo "Usage: $command [workingDir]"
exit 1
}
-# cleanup the temporary directory when exiting the script
-function cleanup() {
- if [ -n "$TMP_DIR" ] && [ -d "$TMP_DIR" ] && [ "$workingDir" = "$TMP_DIR" ] ; then
- echo "The temporary directory $TMP_DIR needs to be cleaned up."
- fi
-}
-
-# This function builds Pinot given a specific commit hash and target directory
-function checkoutAndBuild() {
- commitHash=$1
- targetDir=$2
-
- pushd "$targetDir" || exit 1
- git init
- git remote add origin https://github.com/apache/incubator-pinot
- git fetch --depth 1 origin "$commitHash"
- git checkout FETCH_HEAD
- mvn install package -DskipTests -Pbin-dist
- popd || exit 1
-}
-
# Given a component and directory, start that version of the specific component
function startService() {
serviceName=$1
dirName=$2
# Upon start, save the pid of the process for a component into a file in /tmp/{component}.pid, which is then used to stop it
pushd "$dirName"/pinot-tools/target/pinot-tools-pkg/bin || exit 1
if [ "$serviceName" = "zookeeper" ]; then
- sh -c 'echo $$ > $0/zookeeper.pid; exec ./pinot-admin.sh StartZookeeper' "${dirName}" &
+ sh -c 'rm -rf /tmp/zkdir'
+ sh -c 'echo $$ > $0/zookeeper.pid; exec ./pinot-admin.sh StartZookeeper -dataDir /tmp/zkdir > ${0}/zookeeper.log 2>&1' "${dirName}" &
elif [ "$serviceName" = "controller" ]; then
- sh -c 'echo $$ > $0/controller.pid; exec ./pinot-admin.sh StartController' "${dirName}" &
+ sh -c 'echo $$ > $0/controller.pid; exec ./pinot-admin.sh StartController > ${0}/controller.log 2>&1' "${dirName}" &
elif [ "$serviceName" = "broker" ]; then
- sh -c 'echo $$ > $0/broker.pid; exec ./pinot-admin.sh StartBroker' "${dirName}" &
+ sh -c 'echo $$ > $0/broker.pid; exec ./pinot-admin.sh StartBroker > ${0}/broker.log 2>&1' "${dirName}" &
elif [ "$serviceName" = "server" ]; then
- sh -c 'echo $$ > $0/server.pid; exec ./pinot-admin.sh StartServer' "${dirName}" &
+ sh -c 'echo $$ > $0/server.pid; exec ./pinot-admin.sh StartServer > ${0}/server.log 2>&1' "${dirName}" &
elif [ "$serviceName" = "kafka" ]; then
- sh -c 'echo $$ > $0/kafka.pid; exec ./pinot-admin.sh StartKafka -zkAddress localhost:2181/kafka' "${dirName}" &
+ sh -c 'echo $$ > $0/kafka.pid; exec ./pinot-admin.sh StartKafka -zkAddress localhost:2181/kafka > ${0}/kafka.log 2>&1' "${dirName}" &
fi
+
+ # sleep 60s to allow service to do some initialization:
+ # 1. controller depends on zookeeper, if not wait zookeeper to be ready, controller will crash.
+ # 2. broker depends on controller, if not wait controller to be ready, broker will crash.
Review comment:
you should include these sleeps in the start services method. Do the following:
- start Zookeeper
- call waitForZkReady
- start controller
- call waitForControllerReady
- start server
- start broker
- call waitForClusterReady
For now, in each of the callForXXXReady methods, just put a sleep for 10s, and add a TODO. We will add a check for zk, controller, etc. maybe in java via yaml file or using shell commands later.
##########
File path: compatibility-verifier/compCheck.sh
##########
@@ -217,29 +150,87 @@ if [ "$(lsof -t -i:8097 -s TCP:LISTEN)" ] || [ "$(lsof -t -i:8098 -sTCP:LISTEN)"
exit 1
fi
-# Setup initial cluster with olderCommit and do rolling upgrade
+# Setup initial cluster with olderCommit
startServices "$oldTargetDir"
-#$COMPAT_TESTER pre-controller-upgrade.yaml; if [ $? -ne 0 ]; then exit 1; fi
+
+$COMPAT_TESTER "offline-table-create-op.yaml" 1; if [ $? -ne 0 ]; then exit 1; fi
Review comment:
Don't add these yet. My plan is to change the compat-tester to take a dir name and execute all yamls under the directory. For now, just leaving one call commented as a placeholder is enough.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu merged pull request #6650: complete compatibility regression testing
Posted by GitBox <gi...@apache.org>.
mcvsubbu merged pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mqliang closed pull request #6650: complete compatibility regression testing
Posted by GitBox <gi...@apache.org>.
mqliang closed pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on pull request #6650: complete compatibility regression testing
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650#issuecomment-791657823
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=h1) Report
> Merging [#6650](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=desc) (0064dc4) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `0.62%`.
> The diff coverage is `62.64%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6650 +/- ##
==========================================
- Coverage 66.44% 65.81% -0.63%
==========================================
Files 1075 1360 +285
Lines 54773 66533 +11760
Branches 8168 9697 +1529
==========================================
+ Hits 36396 43792 +7396
- Misses 15700 19620 +3920
- Partials 2677 3121 +444
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests | `65.81% <62.64%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `0.00% <0.00%> (-79.32%)` | :arrow_down: |
| [...pinot/broker/api/resources/PinotClientRequest.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdENsaWVudFJlcXVlc3QuamF2YQ==) | `0.00% <0.00%> (-27.28%)` | :arrow_down: |
| [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `71.42% <ø> (-28.58%)` | :arrow_down: |
| [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `33.96% <0.00%> (-32.71%)` | :arrow_down: |
| [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `66.66% <ø> (+9.52%)` | :arrow_up: |
| [...n/java/org/apache/pinot/client/BrokerResponse.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Jyb2tlclJlc3BvbnNlLmphdmE=) | `100.00% <ø> (ø)` | |
| [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `35.55% <ø> (-13.29%)` | :arrow_down: |
| [...org/apache/pinot/client/DynamicBrokerSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0R5bmFtaWNCcm9rZXJTZWxlY3Rvci5qYXZh) | `82.85% <ø> (+10.12%)` | :arrow_up: |
| [...n/java/org/apache/pinot/client/ExecutionStats.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4ZWN1dGlvblN0YXRzLmphdmE=) | `68.88% <ø> (ø)` | |
| ... and [1241 more](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=footer). Last update [b2d716d...0064dc4](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6650: complete compatibility regression testing
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650#issuecomment-791657823
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=h1) Report
> Merging [#6650](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=desc) (9f8507c) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `22.28%`.
> The diff coverage is `42.85%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6650 +/- ##
===========================================
- Coverage 66.44% 44.16% -22.29%
===========================================
Files 1075 1360 +285
Lines 54773 66537 +11764
Branches 8168 9701 +1533
===========================================
- Hits 36396 29388 -7008
- Misses 15700 34693 +18993
+ Partials 2677 2456 -221
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `44.16% <42.85%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...t/broker/broker/BasicAuthAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jhc2ljQXV0aEFjY2Vzc0NvbnRyb2xGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
| [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
| [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...ceselector/StrictReplicaGroupInstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL1N0cmljdFJlcGxpY2FHcm91cEluc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...roker/routing/segmentpruner/TimeSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1RpbWVTZWdtZW50UHJ1bmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...roker/routing/segmentpruner/interval/Interval.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL2ludGVydmFsL0ludGVydmFsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| ... and [1383 more](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=footer). Last update [b2d716d...9f8507c](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mqliang commented on a change in pull request #6650: complete compatibility regression testing
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650#discussion_r591077674
##########
File path: compatibility-verifier/compCheck.sh
##########
@@ -34,61 +34,45 @@
# and run all the scripts in the directory in alpha order, one script at each
# "stage" of upgrade.
#
-# We may modify to choose a minimal run in which the same set of operatons are run
+# We may modify to choose a minimal run in which the same set of operations are run
# between any two component upgrades/rollbacks -- this may consist of adding
# one more segment to table, adding some more rows to the stream topic, and
# running some queries with the new data.
-# get a temporary directory in case the workingDir is not provided by user
-TMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'mytmpdir')
-
-COMPAT_TESTER_PATH="pinot-integration-tests/target/pinot-integration-tests-pkg/bin/pinot-compat-test-runner.sh"
# get usage of the script
function usage() {
command=$1
- echo "Usage: $command olderCommit newerCommit [workingDir]"
+ echo "Usage: $command [workingDir]"
exit 1
}
-# cleanup the temporary directory when exiting the script
-function cleanup() {
- if [ -n "$TMP_DIR" ] && [ -d "$TMP_DIR" ] && [ "$workingDir" = "$TMP_DIR" ] ; then
- echo "The temporary directory $TMP_DIR needs to be cleaned up."
- fi
-}
-
-# This function builds Pinot given a specific commit hash and target directory
-function checkoutAndBuild() {
- commitHash=$1
- targetDir=$2
-
- pushd "$targetDir" || exit 1
- git init
- git remote add origin https://github.com/apache/incubator-pinot
- git fetch --depth 1 origin "$commitHash"
- git checkout FETCH_HEAD
- mvn install package -DskipTests -Pbin-dist
- popd || exit 1
-}
-
# Given a component and directory, start that version of the specific component
function startService() {
serviceName=$1
dirName=$2
# Upon start, save the pid of the process for a component into a file in /tmp/{component}.pid, which is then used to stop it
pushd "$dirName"/pinot-tools/target/pinot-tools-pkg/bin || exit 1
if [ "$serviceName" = "zookeeper" ]; then
- sh -c 'echo $$ > $0/zookeeper.pid; exec ./pinot-admin.sh StartZookeeper' "${dirName}" &
+ sh -c 'rm -rf /tmp/zkdir'
+ sh -c 'echo $$ > $0/zookeeper.pid; exec ./pinot-admin.sh StartZookeeper -dataDir /tmp/zkdir > ${0}/zookeeper.log 2>&1' "${dirName}" &
elif [ "$serviceName" = "controller" ]; then
- sh -c 'echo $$ > $0/controller.pid; exec ./pinot-admin.sh StartController' "${dirName}" &
+ sh -c 'echo $$ > $0/controller.pid; exec ./pinot-admin.sh StartController > ${0}/controller.log 2>&1' "${dirName}" &
elif [ "$serviceName" = "broker" ]; then
- sh -c 'echo $$ > $0/broker.pid; exec ./pinot-admin.sh StartBroker' "${dirName}" &
+ sh -c 'echo $$ > $0/broker.pid; exec ./pinot-admin.sh StartBroker > ${0}/broker.log 2>&1' "${dirName}" &
elif [ "$serviceName" = "server" ]; then
- sh -c 'echo $$ > $0/server.pid; exec ./pinot-admin.sh StartServer' "${dirName}" &
+ sh -c 'echo $$ > $0/server.pid; exec ./pinot-admin.sh StartServer > ${0}/server.log 2>&1' "${dirName}" &
elif [ "$serviceName" = "kafka" ]; then
- sh -c 'echo $$ > $0/kafka.pid; exec ./pinot-admin.sh StartKafka -zkAddress localhost:2181/kafka' "${dirName}" &
+ sh -c 'echo $$ > $0/kafka.pid; exec ./pinot-admin.sh StartKafka -zkAddress localhost:2181/kafka > ${0}/kafka.log 2>&1' "${dirName}" &
fi
+
+ # sleep 60s to allow service to do some initialization:
+ # 1. controller depends on zookeeper, if not wait zookeeper to be ready, controller will crash.
+ # 2. broker depends on controller, if not wait controller to be ready, broker will crash.
Review comment:
done
##########
File path: compatibility-verifier/compCheck.sh
##########
@@ -217,29 +150,87 @@ if [ "$(lsof -t -i:8097 -s TCP:LISTEN)" ] || [ "$(lsof -t -i:8098 -sTCP:LISTEN)"
exit 1
fi
-# Setup initial cluster with olderCommit and do rolling upgrade
+# Setup initial cluster with olderCommit
startServices "$oldTargetDir"
-#$COMPAT_TESTER pre-controller-upgrade.yaml; if [ $? -ne 0 ]; then exit 1; fi
+
+$COMPAT_TESTER "offline-table-create-op.yaml" 1; if [ $? -ne 0 ]; then exit 1; fi
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6650: complete compatibility regression testing
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650#discussion_r591699572
##########
File path: compatibility-verifier/compCheck.sh
##########
@@ -105,13 +102,18 @@ function stopService() {
else
echo "Pid file ${dirName}/${serviceName}.pid not found. Failed to stop component ${serviceName}"
fi
+ echo "${serviceName} stopped"
}
# Starts a Pinot cluster given a specific target directory
function startServices() {
dirName=$1
startService zookeeper "$dirName"
+ # Controller depends on zookeeper, if not wait zookeeper to be ready, controller will crash.
+ waitForZkReady
startService controller "$dirName"
+ # Broker depends on controller, if not wait controller to be ready, broker will crash.
+ waitForControllerReady
Review comment:
Add a `waitForClusterReady` after line 118.
It may also be useful to add a `waitForKafkaReady` after line 119 since there may be some race conditions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mqliang commented on a change in pull request #6650: complete compatibility regression testing
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650#discussion_r591077603
##########
File path: compatibility-verifier/compCheck.sh
##########
@@ -34,61 +34,45 @@
# and run all the scripts in the directory in alpha order, one script at each
# "stage" of upgrade.
#
-# We may modify to choose a minimal run in which the same set of operatons are run
+# We may modify to choose a minimal run in which the same set of operations are run
# between any two component upgrades/rollbacks -- this may consist of adding
# one more segment to table, adding some more rows to the stream topic, and
# running some queries with the new data.
-# get a temporary directory in case the workingDir is not provided by user
-TMP_DIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'mytmpdir')
-
-COMPAT_TESTER_PATH="pinot-integration-tests/target/pinot-integration-tests-pkg/bin/pinot-compat-test-runner.sh"
# get usage of the script
function usage() {
command=$1
- echo "Usage: $command olderCommit newerCommit [workingDir]"
+ echo "Usage: $command [workingDir]"
exit 1
}
-# cleanup the temporary directory when exiting the script
-function cleanup() {
- if [ -n "$TMP_DIR" ] && [ -d "$TMP_DIR" ] && [ "$workingDir" = "$TMP_DIR" ] ; then
- echo "The temporary directory $TMP_DIR needs to be cleaned up."
- fi
-}
-
-# This function builds Pinot given a specific commit hash and target directory
-function checkoutAndBuild() {
- commitHash=$1
- targetDir=$2
-
- pushd "$targetDir" || exit 1
- git init
- git remote add origin https://github.com/apache/incubator-pinot
- git fetch --depth 1 origin "$commitHash"
- git checkout FETCH_HEAD
- mvn install package -DskipTests -Pbin-dist
- popd || exit 1
-}
-
# Given a component and directory, start that version of the specific component
function startService() {
serviceName=$1
dirName=$2
# Upon start, save the pid of the process for a component into a file in /tmp/{component}.pid, which is then used to stop it
pushd "$dirName"/pinot-tools/target/pinot-tools-pkg/bin || exit 1
if [ "$serviceName" = "zookeeper" ]; then
- sh -c 'echo $$ > $0/zookeeper.pid; exec ./pinot-admin.sh StartZookeeper' "${dirName}" &
+ sh -c 'rm -rf /tmp/zkdir'
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6650: complete compatibility regression testing
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6650:
URL: https://github.com/apache/incubator-pinot/pull/6650#issuecomment-791657823
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=h1) Report
> Merging [#6650](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=desc) (96cf993) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `22.43%`.
> The diff coverage is `42.62%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6650 +/- ##
===========================================
- Coverage 66.44% 44.01% -22.44%
===========================================
Files 1075 1361 +286
Lines 54773 66523 +11750
Branches 8168 9702 +1534
===========================================
- Hits 36396 29279 -7117
- Misses 15700 34796 +19096
+ Partials 2677 2448 -229
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `44.01% <42.62%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...t/broker/broker/BasicAuthAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0Jhc2ljQXV0aEFjY2Vzc0NvbnRyb2xGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
| [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
| [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...ceselector/StrictReplicaGroupInstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL1N0cmljdFJlcGxpY2FHcm91cEluc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...roker/routing/segmentpruner/TimeSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1RpbWVTZWdtZW50UHJ1bmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...roker/routing/segmentpruner/interval/Interval.java](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL2ludGVydmFsL0ludGVydmFsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| ... and [1387 more](https://codecov.io/gh/apache/incubator-pinot/pull/6650/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=footer). Last update [b2d716d...96cf993](https://codecov.io/gh/apache/incubator-pinot/pull/6650?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org