anmolnar commented on PR #8809:
URL: https://github.com/apache/hbase/pull/8809#issuecomment-6838196146
> _Co-authored by Claude (Anthropic) on behalf of @anmolnar_
Thanks @kgeisz, this looks much better. Moving the configs to a scratch dir
and naming everything from `BUILD_TAG` takes care of the two things I was most
worried about, and the quoted heredoc for `hbase shell` is a nice fix. A few
things I noticed in the update:
1. **`_kill_orphaned_processes` won't match anything.** The image is a
binary install (`lib/ruby` exists), so `bin/hbase` starts the shell as
`org.jruby.JarBootstrapMain`, not `org.jruby.Main`. The `pkill -f
'org.jruby.Main.*shell'` pattern finds nothing and the timed-out shell keeps
running. Matching on `JarBootstrapMain` should fix it.
2. **`list_tables` returning `[]` when it can't parse the output.** This
makes `assert_table_does_not_exist` pass whenever the shell output is
unexpected (an error, a startup failure, a format change). I'd raise an
exception that includes the raw output instead, so a bad result fails loudly.
3. **`zoo.cfg` is no longer read.** `_replace_hostnames_in_configs` only
rewrites `hbase-site.xml`, so in CI the copied `zoo.cfg` still says
`hbase-docker`. That's harmless, because master doesn't read `zoo.cfg` at all
(nothing in the Java sources refers to it). But that means
`cluster{1,2}/conf/zoo.cfg` are dead files that make it look like they matter.
Can we delete them, or rewrite them too if I've missed somewhere they're used?
4. **The README says local runs use `hbase-docker` / `hbase-docker-2`,** but
`run_read_replica_integration_tests.sh` sets `HBASE_CONTAINER_NAME=hbase-local`
when `BUILD_TAG` is unset. Only running pytest directly gets the `.env`
default. Small doc fix.
5. **Hostname length (minor).** Container names and hostnames come from the
full `BUILD_TAG`, e.g. `hbase-jenkins-HBase-Read-Replica-Tests-<branch>-<n>-2`.
A long branch name can push that past the 63-character limit for a hostname.
Truncating it, or using a short hash of `BUILD_TAG`, would avoid that.
6. **Leftover stacks (optional).** Because each build now gets a unique
project name, a stack leaked by an earlier SIGKILLed build won't collide with
later ones, but nothing removes it either. Removing leftover containers whose
name starts with `rr-jenkins-` at the start of each run would keep the agent
clean.
Items 1 and 2 are the ones I'd want fixed in this PR. The rest, plus the
earlier follow-ups (single-stage Dockerfile and `.m2` cache, `-j` doing
nothing, sleeps and blanket reruns, the docker packages in the shared CI
image), are fine as a follow-up JIRA if you can link it here. With that I'm
happy to re-approve.
--
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]
