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

Can we improve MMapDir's exceptions for invalid offsets? #11912

Closed
rmuir opened this issue Nov 9, 2022 · 4 comments · Fixed by #11918
Closed

Can we improve MMapDir's exceptions for invalid offsets? #11912

rmuir opened this issue Nov 9, 2022 · 4 comments · Fixed by #11918

Comments

@rmuir
Copy link
Member

rmuir commented Nov 9, 2022

Description

For #11905 bug, as an example, the user may get a generic exception "Seek Past EOF".

Can we improve it to include the bogus position? For example, If you can see that offset is negative, it is faster to debug. I think the new MMapDir already has some improvements here. Better error messages would help not just people debugging real world problems, but also developers making new stuff and debugging tests.

cc: @uschindler

@uschindler
Copy link
Contributor

uschindler commented Nov 9, 2022

As discussed in chat an hour ago:

For MemorySegmentIndexInput it already has a reworked Exception code where also those suppress unused warnings are gone due to some trick: we just catch the exception, but pass them to a method that maps it to an user readable Lucene exception and regrow it This reduced code duplication and removed most suppress warnings.

The code in memory segments already contains a message to report negative positions. I can change it to show the actual position.

I would backport the exception factory methods to the byte buffer index input, that's a bit mechanical work but makes the code also better readable.

@uschindler uschindler self-assigned this Nov 9, 2022
@uschindler uschindler added this to the 9.5.0 milestone Nov 9, 2022
@uschindler
Copy link
Contributor

Maybe include that in 9.4.3 ?

@uschindler
Copy link
Contributor

Will do that tomorrow, PR will come. We can decide to add it to 9.4 branch to make later debugging.of broken code with recent vectors easier.

@rmuir
Copy link
Member Author

rmuir commented Nov 11, 2022

Thanks @uschindler !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants