Re: [DISCUSS] PIP-272 Add a `StateStoreConfig` to the `WorkerConfig`
thanks for the improvements, +1 On Tue, May 30, 2023 at 2:20 AM Pengcheng Jiang wrote: > Hi Mesika: > > Thanks for the suggestions, I updated the pip, and for the rest questions: > > 5. yes, all config goes through arguments instead of a file > 6. it should be a JSON string that can be deserialized to a `Map Object>`, updated in pip > 7. it should be `pulsar-admin functions localrun` command, updated in pip > 8. the `stateStorageServiceUrl` won't be touched > > Sincerely > Pengcheng Jiang > > Asaf Mesika 于2023年5月29日周一 19:53写道： > > > Hi Pengcheng, > > > > Looks like a solid improvement, definitely helping people using their own > > state store. > > > > I have a few comments: > > > > 1. Background knowledge should explain what is a state storage > > 2. Move problem description from Background Knowledge to Motivation. > > > > I'm quoting the template to understand what should be included in > > the Background knowledge section: > > > > > > > > 3. `WorkerConfig` - explain briefly what is Worker and how it differs > from > > Broker. Should be in background knowledge section. > > > > 4. Background knowledge should explain briefly what is a runtime and > > runtime factory. > > > > 5. > > > > Add a new cli argument to JavaInstanceStarter and LocalRunner so > > > process runtime can pass state related config to them > > > > > > Today all config goes through arguments and not a file? > > > > 6. `--stateStorageConfig` > > What format is the expected value? > > > > 7. `functions local run` > > What is this? > > > > 8. Are you keeping `stateStorageServiceUrl`? Maybe people rely on it? > > > > 9. Don't forget to include link to discussion thread using Apache Pony > Mail > > > > > > On Mon, May 29, 2023 at 10:44 AM Rui Fu wrote: > > > > > Hi Pengcheng, > > > > > > Thanks for bringing this up, the PIP lgtm, +1. > > > > > > Best, > > > > > > Rui Fu > > > On May 29, 2023 at 13:52 +0800, Enrico Olivelli , > > > wrote: > > > > Looks good > > > > +1 > > > > > > > > Enrico > > > > > > > > Il Lun 29 Mag 2023, 04:47 Pengcheng Jiang > > > > ha scritto: > > > > > > > > > Dear Pulsar community, > > > > > > > > > > I created a pip to make pulsar functions' `StateStoreProvider` > > > configurable > > > > > with custom configurations: > > > https://github.com/apache/pulsar/issues/20419 > > > > > > > > > > Any feedback and suggestions are welcome > > > > > > > > > > Sincerely > > > > > Pengcheng Jiang > > > > > > > > > > >
PLEASE NOTE - from now on PIPs are PR based
Hi, I've completed all steps necessary for implementing "PIP-265: PR-based system for managing and reviewing PIPs" ( https://github.com/apache/pulsar/issues/20207). PLEASE NOTE, from now on, PIPs are to be submitted *through Pull Request*. You can read the process detail here: https://github.com/apache/pulsar/blob/master/pip/README.md I've marked the PIP issue deprecated and if you're using it, you'll see instructions to view the new PIP process, so I don't anticipate people will submit PIP through GitHub issues anymore - thanks Michael for that suggestion. Let's try to help future contributors to follow the new process. Thanks! Asaf
Re: [DISCUSS] PIP-272 Add a `StateStoreConfig` to the `WorkerConfig`
Hi Mesika: Thanks for the suggestions, I updated the pip, and for the rest questions: 5. yes, all config goes through arguments instead of a file 6. it should be a JSON string that can be deserialized to a `Map`, updated in pip 7. it should be `pulsar-admin functions localrun` command, updated in pip 8. the `stateStorageServiceUrl` won't be touched Sincerely Pengcheng Jiang Asaf Mesika 于2023年5月29日周一 19:53写道： > Hi Pengcheng, > > Looks like a solid improvement, definitely helping people using their own > state store. > > I have a few comments: > > 1. Background knowledge should explain what is a state storage > 2. Move problem description from Background Knowledge to Motivation. > > I'm quoting the template to understand what should be included in > the Background knowledge section: > > > > 3. `WorkerConfig` - explain briefly what is Worker and how it differs from > Broker. Should be in background knowledge section. > > 4. Background knowledge should explain briefly what is a runtime and > runtime factory. > > 5. > > Add a new cli argument to JavaInstanceStarter and LocalRunner so > > process runtime can pass state related config to them > > > Today all config goes through arguments and not a file? > > 6. `--stateStorageConfig` > What format is the expected value? > > 7. `functions local run` > What is this? > > 8. Are you keeping `stateStorageServiceUrl`? Maybe people rely on it? > > 9. Don't forget to include link to discussion thread using Apache Pony Mail > > > On Mon, May 29, 2023 at 10:44 AM Rui Fu wrote: > > > Hi Pengcheng, > > > > Thanks for bringing this up, the PIP lgtm, +1. > > > > Best, > > > > Rui Fu > > On May 29, 2023 at 13:52 +0800, Enrico Olivelli , > > wrote: > > > Looks good > > > +1 > > > > > > Enrico > > > > > > Il Lun 29 Mag 2023, 04:47 Pengcheng Jiang > > > ha scritto: > > > > > > > Dear Pulsar community, > > > > > > > > I created a pip to make pulsar functions' `StateStoreProvider` > > configurable > > > > with custom configurations: > > https://github.com/apache/pulsar/issues/20419 > > > > > > > > Any feedback and suggestions are welcome > > > > > > > > Sincerely > > > > Pengcheng Jiang > > > > > > >
Re: [DISUCSS] Fix and republish the pulsar-all image for Pulsar 3.0.0
Hi, Enrico > When we ran the VOTE and we provided the docker images, were they already broken ? Actually, they are not broken unless we use the new features of Pulsar 3.0.0. I think we need something like verification test scripts, to verify the release candidate. For example, we use the image provided in the RC to run the integration tests. And we need to make sure that we have tested the newly added feature for the RC docker image. What do you think? BR, Zike Yang On Tue, May 30, 2023 at 3:14 PM Zike Yang wrote: > > Hi, Asaf > > > How do you suggest we prevent it from happening next time? > > I have pushed a PR to fix it: https://github.com/apache/pulsar/pull/20435 > This PR specifies the correct image name for `pulsar` image to build > pulsar-all. > > Note that, in the release of Pulsar 3.0, we build the docker image by > executing the following command instead of the `docker/build.sh`: > ``` > mvn install -DUBUNTU_MIRROR=http://azure.archive.ubuntu.com/ubuntu/ \ > -DskipTests \ > -Pdocker -Pdocker-push \ > -Ddocker.platforms=linux/amd64,linux/arm64 \ > -Ddocker.organization=snzkyang \ > -pl docker/pulsar,docker/pulsar-all > ``` > I think to take it a step further, we could fix these scripts(build.sh > and publish.sh) and use the shell scripts to build the image. > > I have verified the PR, and it works well. Please see more detail in > the PR description. > > Thanks, > Zike Yang > > On Mon, May 29, 2023 at 9:50 PM Enrico Olivelli wrote: > > > > I am really worried about the process. > > > > When we ran the VOTE and we provided the docker images, were they > > already broken ? > > > > In any case we cannot overwrite those images, they have been cached > > all over the world now. > > > > It is safer to cut a new 3.0.1 release and run a VOTE. > > > > Maybe we can remove the old images, forever > > > > Enrico > > > > Il giorno lun 29 mag 2023 alle ore 13:55 Asaf Mesika > > ha scritto: > > > > > > Good catch! > > > > > > How do you suggest we prevent it from happening next time? > > > > > > On Mon, May 29, 2023 at 1:34 PM Zike Yang wrote: > > > > > > > Hi, all > > > > > > > > Recently, we found an issue with the `pulsar-all:3.0.0` image. The > > > > pulsar library included in `pulsar-all:3.0.0` is the version of > > > > 2.11.0: > > > > > > > > ``` > > > > docker run apachepulsar/pulsar-all:3.0.0 ls lib/ | grep pulsar-broker > > > > > > > > org.apache.pulsar-pulsar-broker-2.11.0.jar > > > > org.apache.pulsar-pulsar-broker-auth-sasl-2.11.0.jar > > > > org.apache.pulsar-pulsar-broker-common-2.11.0.jar > > > > ``` > > > > > > > > The root cause is that we use `apachepulsar/pulsar:latest` to build > > > > the `pulsar-all` image. But at the time of building Pulsar 3.0.0, > > > > `apachepulsar/pulsar:latest` was pointing to version 2.11.0. > > > > > > > > Therefore, the `pulsar-all:3.0.0` is actually a version 2.11.0 of > > > > Pulsar but with 3.0.0 connectors and offloaders. > > > > > > > > Please see more detail in this issue: > > > > https://github.com/apache/pulsar/issues/20420 > > > > > > > > I have rebuilt the `pulsar-all:3.0.0` image: > > > > > > > > https://hub.docker.com/layers/snzkyang/pulsar-all/3.0.0/images/sha256-833ea988bce8c704b179cc4c9c38fac8980e108b0bc67454e06c22927990b169?context=explore > > > > > > > > Please help and verify it. And check if there are any other problems > > > > with the image. > > > > > > > > I'm going to publish the image to the `apachepulsar` organization to > > > > replace the old one. But before we do that, do we need a Vote or other > > > > ways to reach a consensus? Is there any problem if we replace the old > > > > image? > > > > > > > > Besides, I will also fix the docker build script to avoid similar > > > > issues. > > > > > > > > Thanks, > > > > Zike Yang > > > >
Re: [DISUCSS] Fix and republish the pulsar-all image for Pulsar 3.0.0
Hi, Asaf > How do you suggest we prevent it from happening next time? I have pushed a PR to fix it: https://github.com/apache/pulsar/pull/20435 This PR specifies the correct image name for `pulsar` image to build pulsar-all. Note that, in the release of Pulsar 3.0, we build the docker image by executing the following command instead of the `docker/build.sh`: ``` mvn install -DUBUNTU_MIRROR=http://azure.archive.ubuntu.com/ubuntu/ \ -DskipTests \ -Pdocker -Pdocker-push \ -Ddocker.platforms=linux/amd64,linux/arm64 \ -Ddocker.organization=snzkyang \ -pl docker/pulsar,docker/pulsar-all ``` I think to take it a step further, we could fix these scripts(build.sh and publish.sh) and use the shell scripts to build the image. I have verified the PR, and it works well. Please see more detail in the PR description. Thanks, Zike Yang On Mon, May 29, 2023 at 9:50 PM Enrico Olivelli wrote: > > I am really worried about the process. > > When we ran the VOTE and we provided the docker images, were they > already broken ? > > In any case we cannot overwrite those images, they have been cached > all over the world now. > > It is safer to cut a new 3.0.1 release and run a VOTE. > > Maybe we can remove the old images, forever > > Enrico > > Il giorno lun 29 mag 2023 alle ore 13:55 Asaf Mesika > ha scritto: > > > > Good catch! > > > > How do you suggest we prevent it from happening next time? > > > > On Mon, May 29, 2023 at 1:34 PM Zike Yang wrote: > > > > > Hi, all > > > > > > Recently, we found an issue with the `pulsar-all:3.0.0` image. The > > > pulsar library included in `pulsar-all:3.0.0` is the version of > > > 2.11.0: > > > > > > ``` > > > docker run apachepulsar/pulsar-all:3.0.0 ls lib/ | grep pulsar-broker > > > > > > org.apache.pulsar-pulsar-broker-2.11.0.jar > > > org.apache.pulsar-pulsar-broker-auth-sasl-2.11.0.jar > > > org.apache.pulsar-pulsar-broker-common-2.11.0.jar > > > ``` > > > > > > The root cause is that we use `apachepulsar/pulsar:latest` to build > > > the `pulsar-all` image. But at the time of building Pulsar 3.0.0, > > > `apachepulsar/pulsar:latest` was pointing to version 2.11.0. > > > > > > Therefore, the `pulsar-all:3.0.0` is actually a version 2.11.0 of > > > Pulsar but with 3.0.0 connectors and offloaders. > > > > > > Please see more detail in this issue: > > > https://github.com/apache/pulsar/issues/20420 > > > > > > I have rebuilt the `pulsar-all:3.0.0` image: > > > > > > https://hub.docker.com/layers/snzkyang/pulsar-all/3.0.0/images/sha256-833ea988bce8c704b179cc4c9c38fac8980e108b0bc67454e06c22927990b169?context=explore > > > > > > Please help and verify it. And check if there are any other problems > > > with the image. > > > > > > I'm going to publish the image to the `apachepulsar` organization to > > > replace the old one. But before we do that, do we need a Vote or other > > > ways to reach a consensus? Is there any problem if we replace the old > > > image? > > > > > > Besides, I will also fix the docker build script to avoid similar issues. > > > > > > Thanks, > > > Zike Yang > > >
