gortiz commented on PR #19448: URL: https://github.com/apache/pinot/pull/19448#issuecomment-5539063229
## Verification from the built distribution Everything below was run from the assembled binary distribution (`apache-pinot-*-bin.tar.gz`), not from the source tree, so it exercises the real `plugins/` + `plugin-libs/` layout and the generated launcher scripts. ### Quickstarts Run sequentially, each with a clean cluster: | Type | Completed | Tables | Classloading errors | |---|---|---|---| | `batch` | yes | 15 | 0 | | `MATERIALIZED_VIEW` | yes | 2 | 0 | | `TPCH` | yes | 8 | 0 | | `LOGICAL_TABLE` | yes | 3 | 0 | | `NULL_HANDLING` | yes | 1 | 0 | | `OFFLINE_MULTIDIR` | yes | 1 | 0 | | `streaming` (managed Kafka) | yes | 12 | 0 | | `GEOSPATIAL` | no | 0 | 0 | | `SORTED` | no | 0 | 0 | No classloading or plugin-resolution failure in any run. `MATERIALIZED_VIEW` is the one that matters most for this PR: it is the only quickstart that drives `pinot-minion-builtin-tasks`, whose dependency on `pinot-materialized-view` this PR changes to `provided`, so those classes must now resolve from `lib/pinot-all.jar` at runtime rather than from the plugin. The `streaming` run matters for the exclusions: `pinot-kafka-3.0` is excluded from `pinot-all.jar` by this PR (kafka classes in the uber-jar go from 5024 to 0), so `KafkaServerStartable` and the consumer must load from `plugins/pinot-stream-ingestion/pinot-kafka-3.0/` alone. It started a broker, created topics, ingested and answered queries. The two that did not complete are pre-existing and unrelated: - `GEOSPATIAL` — `GeoSpatialQuickStart.DATA_DIRECTORIES` is `"examples/batch/starbucksStores/"` with a trailing slash, and `QuickStartBase.copyResourceTableToTmpDirectory` builds the lookup as `sourcePath + File.separator + tableName + "_schema.json"`, so it asks the classloader for a path containing `//` and gets null: `NullPointerException: Missing schema json file for table - starbucksStores`. It is the only quickstart with a trailing slash there, and `batch` loads the same table fine. - `SORTED` — works as documented. `SortedColumnQuickstart`'s own javadoc says the content must be generated first by running `SortedTable`'s main method, and the shipped `examples/batch/sorted/` contains only `ingestionJobSpec.yaml`, so it cannot run from a distribution at all. Neither is covered by CI, which runs `BATCH`, `quick-start-hybrid` and `quick-start-streaming` only. ### Kafka plugins | | Result | |---|---| | `pinot-kafka-base` | pass | | `pinot-kafka-3.0` module tests | 61 run, 0 failures | | `pinot-kafka-4.0` module tests | 44 run, 0 failures | | `streaming` quickstart from the distribution | pass, as above | `pinot-kafka-4.0` is not in the distribution assembly (unchanged by this PR), so module tests are the only coverage available for it. ### Which plugins these actually exercise Derived from the record readers the bootstrapped tables declare, not from log lines, since the quickstart log4j configuration suppresses `PluginManager`'s INFO output. Exercised, 7 of the 28 shipped plugins: | Plugin | How | |---|---| | `pinot-avro` | `airlineStats`, `clientSalaryNulls` | | `pinot-csv` | `baseballStats`, `billing`, `clickstreamFunnel`, `dimBaseballTeams`, `starbucksStores` | | `pinot-json` | `githubEvents`, `githubComplexTypeEvents` | | `pinot-parquet` | `fineFoodReviews` (native reader), `testUnnest` (Avro-backed reader) | | `pinot-batch-ingestion-standalone` | every job spec declares `executionFrameworkSpec: name: 'standalone'` | | `pinot-minion-builtin-tasks` | `MATERIALIZED_VIEW` | | `pinot-kafka-3.0` | `streaming` | A metrics plugin is exercised implicitly by every run: `PinotMetricUtils` hard-fails when no `PinotMetricsFactory` initialises, so any cluster that starts at all proves `pinot-yammer` loads from `plugins/` — which this PR also excludes from the uber-jar. Not exercised, 21 plugins: `pinot-orc`, `pinot-thrift`, `pinot-protobuf`, `pinot-confluent-avro`, `pinot-confluent-json`, `pinot-confluent-protobuf`, `pinot-bson`, `pinot-arrow`, `pinot-clp-log`, `pinot-kinesis`, `pinot-pulsar`, `pinot-s3`, `pinot-gcs`, `pinot-adls`, `pinot-hdfs`, `pinot-azure`, `pinot-dropwizard`, `pinot-compound-metrics`, `pinot-segment-writer-file-based`, `pinot-segment-uploader-default`, `pinot-timeseries-m3ql`. Most need external infrastructure or credentials. Their module tests pass, and the class-set comparison in the PR description shows no class went missing from the distribution, but they are not covered end to end here. ### Developer disk usage A side effect worth recording. Identical command on both branches, `clean package -DskipTests` on the `pinot-plugins` reactor, no `-Pbin-dist`, measuring `pinot-plugins/*/*/target`: | | plugin `target` dirs | shaded jars produced | |---|---|---| | master | 890 MB | 31 | | this PR | 173 MB | 2 (Spark and Hadoop batch, kept deliberately) | | difference | **717 MB smaller, -81%** | | The caveat, in the other direction: a `-Pbin-dist` build adds about 590 MB back across those same targets, because `copy-dependencies` writes each plugin's `plugin-libs/` there. So the saving applies to the ordinary build a developer runs, not to distribution builds. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
