10000 Remove IndexHandle by graetzer · Pull Request #10424 · arangodb/arangodb · GitHub
[go: up one dir, main page]

Skip to content

Remove IndexHandle #10424

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

Merged
merged 4 commits into from
Nov 14, 2019
Merged

Remove IndexHandle #10424

merged 4 commits into from
Nov 14, 2019

Conversation

graetzer
Copy link
Contributor

Scope & Purpose

Remove useless index handle code

  • Bug-Fix for devel-branch (i.e. no need for backports?)
  • Strictly new functionality (i.e. a new feature / new option, no need for porting)

Testing & Verification

This change is a trivial rework / code cleanup without any test coverage.

http://jenkins01.arangodb.biz:8080/job/arangodb-matrix-pr/7155/

@graetzer graetzer added this to the devel milestone Nov 13, 2019
@jsteemann
Copy link
Contributor

I was always wondering what this IndexHandle thing was for.

@graetzer graetzer requested a review from dhly-etc November 13, 2019 15:24
@@ -155,25 +155,23 @@ class SharedState {
_callback = std::forward<F>(func);

auto state = _state.load(std::memory_order_acquire);
while (true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you comment on why this seemingly unrelated change here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The while loop seems unnecessary here.
In the code before this change, the while loop can only be repeated if one had also hit the TRI_ASSERT(false); assertion directly before. That means in maintainer mode, there will be at most a single loop iteration. In release mode however, there could have been multiple loop iterations in theory, because the assert will not trigger.
In reality, execution should never reach the TRI_ASSERT(false); location here. We can still leave it in place for debugging, but as it is not supposed to be reached, we don't need to do anything special for release builds, and can still remove the while loop here.

@jsteemann
Copy link
Contributor

@jsteemann jsteemann merged commit d423359 into devel Nov 14, 2019
@jsteemann jsteemann deleted the bug-fix/remove-index-handle branch November 14, 2019 10:34
ObiWahn added a commit that referenced this pull request Nov 14, 2019
…nto feature/switch-some-scripts-to-python3

* 'devel' of https://github.com/arangodb/arangodb: (107 commits)
  fix description of NetworkFeature options
  Bug fix/dont use indexes in progress (#10432)
  rename `lib_libarango_shell` to `libarango_shell`. (#10433)
  fix issues found by cppcheck (#10434)
  Remove IndexHandle (#10424)
  fix invalid assertion (#10429)
  no coordinators left behind (#10422)
  fix compile warning in AnalyzerFeature test
  allow using `RANDOM_TOKEN` AQL function with an argument value of `0`. (#10414)
  centralize cloning functionality for AstNodes (#10430)
  Don't modify a finalized node. (#10419)
  Bug fix/internal issue #651 (#10415)
  show index type in not implemented exception (#10426)
  Parallel SortingGatherExecutor (#10410)
  "run with --log v8=debug" to see the stacktraces of logged error messages (#10373)
  ARANGODB_UPGRADE_DURING_RESTORE env variable. (#10385)
  add optional exclusive writes to rocksdb engine (#10364)
  attempt to make test more deterministic
  Feature/parallel aql phase one 2 (#10408)
  Improve syncer error message (#10403)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0