Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Log] Ignore callstacktrace test for windows #11413

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

ashione
Copy link
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
Contributor

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

@rkooo567 rkooo567 self-assigned this Oct 16, 2020
@ashione
Copy link
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
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
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
Member

kfstorm commented Oct 16, 2020

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

@rkooo567
Copy link
Contributor

+1 for @kfstorm's qusetion.

@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
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
Member Author

ashione commented Oct 19, 2020

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
Labels
@author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants