[PEDSP-14857] Implement for checking error reason by flight in boolean expression tree

[jh0306lim]

No description provided.

[arodionov53]

For experiment I asked CODE.I to analyze arraylist.c. Here the result (part of it):

Potential Improvements:

• Error Handling: The code uses assertions to check for invalid operations (e.g., out-of-bounds access). In production code, it would be better to handle these errors gracefully, possibly returning error codes or using exceptions.

• Memory Leaks: As discussed earlier, there is a potential for memory leaks if users of this code do not properly free memory allocated by arraylist_slice, arraylist_copy, and other functions that create new arraylists. Adding utility functions like arraylist_free and providing clear documentation can help mitigate this risk.

• Performance: The code uses memmove for shifting elements when inserting or removing items. While memmove is generally efficient, it may not be optimal in all cases. For example, if the arraylist is large and only a few elements are being inserted or removed, it might be more efficient to use a data structure that allows for faster insertions and deletions, such as a linked list or a balanced tree.

• Thread Safety: The current implementation does not provide any thread safety mechanisms. If the arraylist is to be used in a multi-threaded environment, proper synchronization (e.g., using mutexes) should be added to prevent data races and ensure consistency.

• Bounds Checking: The code does not perform bounds checking for some operations (e.g., arraylist_set). Adding bounds checking can help prevent undefined behavior due to out-of-bounds access.

• Documentation: While the code includes some comments, adding more detailed documentation for each function (e.g., parameter descriptions, return values, edge cases) would improve readability and maintainability.

Conclusion

The provided code implements a dynamic array (arraylist) with basic functionality for creating, manipulating, and destroying the array. While it is functional, there are several areas where it could be improved, particularly in terms of error handling, memory management, performance, thread safety, and documentation. Addressing these areas would make the code more robust, efficient, and easier to use in various scenarios.

[zh-kim]

Do you think that all suggestions are valid and need enhancement? 😕
In my opinion on CODE.i words, Thread safety, Documentation, Bounds Checking (duplicated with Error Handling) seems not required while other can be considered to enhance ...

• Performance (use linked list? rather than memmove),
• Error Handling (assertion removal, bounds checking)
• Memory Leak (Documentation for making same number alloc. and free)

Although there was a tiny effort to investigate until now, since we have failed to find out the suitable arraylist meeting CODE.i expecation,
Could you please let us know some good arraylist source which we can review and replace curent implementation with itif you have any?

[zh-kim]

Since we've observed low performance issue with new commits (null check instead of assertion, error handlings). they are all reverted for now and will update again after canary test.

[zh-kim]

Since we've observed low performance issue with new commits (null check instead of assertion, error handlings). they are all reverted for now and will update again after canary test.

Since PEDSP-14857-t3 resolving some CODE.i issues shows a bit low performance regarding the grafana result over the last 24h below, which looks unacceptable figures (around longer than 2ms worse in 95th response time) comparing to production pods' metrics, I would like to ask for help in order for us to improve code quality and performance either.
And if there's nothing to improve performance and if resolving CODE.i issues is not that critical, I would like to know if to leave it as it is (to go without resolving issues).

[arodionov53]

The arraylist module is a versatile program designed to work with any data type, which is why it uses void *. However, using void pointers can be unsafe and it's generally better to avoid them when the data type is fixed, as in this case with uint64_t.

Upon analyzing your program, I noticed that you have not utilized the full functionality of the arraylist module. I found only arraylist_add, arraylist_destroy, and arraylist_join being used. For an experiment, I asked CODE.I to create a similar functionality using uint64_t directly. I have attached the file for your reference.

I believe this implementation can serve as a suitable starting point for our needs. Therefore, we do not need to search for additional free software code. However, if you decide to use the arraylist module, please make sure to mention the author and source in the README file.

Thank you for considering this suggestion.
DynamicArray.txt

[zh-kim]

Many thanks for the suggestion
I totally agree that substituting void* with uint64_t is better and it is good to use dynamic array having simplified set of functions.

Although attached images has 2 hours' grafana result each, dynamic array shows better performance (P99 Resp. Times still worse than production but better than PEDSP-14857 using arraylist) without any issue until now.
If there's no issue until next monday KST, I will merge the commit (Canary(PEDSP-14857-t3) dynamic array changes → Dynamic Array merged) here.

[zh-kim]

Since dynamic array shows better result regarding metrics, decided to use it from now on.

[zh-kim]

Fix errors on make build-test-benchmark and unused USE_REASONLIST (which were found late).

[bslee0425]

Thanks for your work on this.