Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23781 )

Change subject: IMPALA-14284: Log the actual log files instead of symlinks in 
start-impala-cluster.py
......................................................................


Patch Set 3:

(1 comment)

> Patch Set 3:
>
> > Patch Set 3:
> >
> > This change is great and very much needed, but my local testing is showing 
> > that it does not work when the cluster is started at the class level 
> > instead of at the function level.  For example, when I hard coded an assert 
> > failure in test_otel_trace.py test_select_timeout 
> > (https://github.com/apache/impala/blob/d4992d532b6fe12bce6dd12f0559b9899a547337/tests/custom_cluster/test_otel_trace.py#L166),
> >  it does not show the "Actual log file names" log messages even if I use 
> > the -k option to limit impala-py.test to only run test_select_timeout.
>
> Correction -- it will show the "Actual log file names" lines if I use -k to 
> run only the test that fails, but if I run other tests that pass, the "Actual 
> log file names" logs are not outputted.

It's not a problem of this script. The issue is due to the cluster only starts 
once for the whole test class TestOtelTraceSelectsDMLs. So the startup logs are 
considered as output of the first test method and only be shown if the first 
test fails. When running with "-k test_select_timeout", it's the only test 
method so it starts the cluster. So we can see the startup logs. When running 
with other test methods and the first one passes, the startup logs are missing.

By adding an "-s" option in the line of "addopts" in tests/pytest.ini, pytest 
will show all outputs (including the passed test). Then we can see the startup 
logs from the first test. But I think we need an better approach since it's too 
verbose. We can improve this in a separate patch.

http://gerrit.cloudera.org:8080/#/c/23781/3/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/23781/3/bin/start-impala-cluster.py@1290
PS3, Line 1290:     if cluster_ops.log_symlinks:
> Something like printing the last few fatal/error log lines seem useful, but
Yeah, a different patch might be better. We need a way to detect if the symlink 
is pointing to the log of a previous run. The current run might fail before 
creating the log file.



--
To view, visit http://gerrit.cloudera.org:8080/23781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id76c0a8bdfb221ab24ee315e2e273abca4257398
Gerrit-Change-Number: 23781
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Wed, 17 Dec 2025 01:58:34 +0000
Gerrit-HasComments: Yes

Reply via email to