Skip to content

[Log] Ignore callstacktrace test for windows#11413

Merged
kfstorm merged 1 commit intoray-project:masterfrom
antgroup:ignore-windows-stack-test
Oct 20, 2020
Merged

[Log] Ignore callstacktrace test for windows#11413
kfstorm merged 1 commit intoray-project:masterfrom
antgroup:ignore-windows-stack-test

Conversation

@ashione
Copy link
Copy Markdown
Member

@ashione ashione commented Oct 15, 2020

Why are these changes needed?

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@ashione ashione requested a review from kfstorm October 15, 2020 12:43
@rkooo567
Copy link
Copy Markdown
Contributor

rkooo567 commented Oct 16, 2020

Why do we do this? Is this not working on Windows?

@rkooo567 rkooo567 self-assigned this Oct 16, 2020
@ashione
Copy link
Copy Markdown
Member Author

ashione commented Oct 16, 2020

Why do we do this? Is this not working on Windows?

Max depth of GetStackStrace is 32, but It will dump more than 32 frames before test case so we can't catch the all stack frames within 32 depth size.

@ashione
Copy link
Copy Markdown
Member Author

ashione commented Oct 16, 2020

Why do we do this? Is this not working on Windows?

Max depth of GetStackStrace is 32, but It will dump more than 32 frames before test case so we can't catch the all stack frames within 32 depth size.

You may find the detail in https://github.com/ray-project/ray/pull/11410/checks?check_run_id=1257791585

@ashione
Copy link
Copy Markdown
Member Author

ashione commented Oct 16, 2020

These are stacktrace on windows. I have no idea about that. It looks like other thread stack not main thread where gtest test case run in.

    @   00007FF669B968C0  public: class std::basic_ostream<char,struct std::char_traits<char> > * __ptr64 __cdecl google::base::CheckOpMessageBuilder::ForVar1(void) __ptr64
    @   00007FF669B956DF  public: virtual void __cdecl google::LogSink::WaitTillSent(void) __ptr64
    @   00007FF669B951BC  public: virtual void __cdecl google::LogSink::WaitTillSent(void) __ptr64
    @   00007FF669BA3D4C  bool __cdecl google::Demangle(char const * __ptr64,char * __ptr64,int)
    @   00007FF669BA3C36  bool __cdecl google::Demangle(char const * __ptr64,char * __ptr64,int)
    @   00007FF669BB54DC  bool __cdecl google::Demangle(char const * __ptr64,char * __ptr64,int)
    @   00007FF669BB527D  bool __cdecl google::Demangle(char const * __ptr64,char * __ptr64,int)
    @   00007FF669BB5D08  bool __cdecl google::Demangle(char const * __ptr64,char * __ptr64,int)
    @   00007FF669BA3DEC  bool __cdecl google::Demangle(char const * __ptr64,char * __ptr64,int)
    @   00007FF669BA3D16  bool __cdecl google::Demangle(char const * __ptr64,char * __ptr64,int)
    @   00007FF669BB571C  bool __cdecl google::Demangle(char const * __ptr64,char * __ptr64,int)
    @   00007FF669B95AAF  public: virtual void __cdecl google::LogSink::WaitTillSent(void) __ptr64
    @   00007FF669BC1760  bool __cdecl google::Demangle(char const * __ptr64,char * __ptr64,int)
    @   00007FF8E8237974  BaseThreadInitThunk
    @   00007FF8E839A271  RtlUserThreadStart

@kfstorm
Copy link
Copy Markdown
Member

kfstorm commented Oct 16, 2020

@ashione Are you suggesting that GetCallTrace doesn't work on Windows?

@rkooo567
Copy link
Copy Markdown
Contributor

rkooo567 commented Oct 16, 2020

+1 for @kfstorm's qusetion.

@rkooo567 rkooo567 mentioned this pull request Oct 16, 2020
Merged
6 tasks
@rkooo567 rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 17, 2020
@ashione
Copy link
Copy Markdown
Member Author

ashione commented Oct 19, 2020

@rkooo567 @kfstorm I'll check code path of glog getcallstack on windows. I suspect this feature was originally unreliable.

@ashione
Copy link
Copy Markdown
Member Author

ashione commented Oct 19, 2020 •
edited
Loading

Stacktrace seems unsupported in glog of ray on windows. I find the glog patch google/glog#168 porting a feature to windows about OS stacktrace. @rkooo567 @kfstorm it's workaround by merging this PR and think about how to get glog's patch in later.

@kfstorm kfstorm merged commit aed739f into ray-project:master Oct 20, 2020
@kfstorm kfstorm deleted the ignore-windows-stack-test branch October 20, 2020 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kfstorm kfstorm kfstorm approved these changes

@rkooo567 rkooo567 rkooo567 approved these changes

Assignees

@rkooo567 rkooo567

Labels

@author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ashione @rkooo567 @kfstorm