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

HBase counters #758

Closed
wants to merge 29 commits into from
Closed

HBase counters #758

wants to merge 29 commits into from

Conversation

pinifaran
Copy link

added support for using hbase counters as the underlying implementation of opentsdb
added support for summing up detected duplicate counters (instead of discarding them)

@pinifaran pinifaran mentioned this pull request Mar 31, 2016
@johann8384
Copy link
Member

This still isn't rebased off the latest Next, did you forget to do a git pull or something?

Do a "git fetch --all; git rebase origin/next" (assuming origin is what you named the OpenTSBD fork, you might have called it something else)

@pinifaran
Copy link
Author

I did everything (fork, create pull request...) from github.com, not from command line.

anyway, i just did what you asked from command line (Git Shell), and now it appears I'm synched:

C:\Users\pini\Documents\GitHub\opentsdb [next]> git fetch --all; git rebase origin/next
Fetching origin
Fetching upstream
Fetching OpenTSDB
First, rewinding head to replay your work on top of it...
Fast-forwarded next to origin/next.

However, i see that there are still a few conflicts:

C:\Users\pini\Documents\GitHub\opentsdb [next]> git merge upstream/next
Removing third_party/hbase/asynchbase-1.7.0-20150910.030815-3.jar.md5
Auto-merging src/utils/Config.java
CONFLICT (content): Merge conflict in src/utils/Config.java
Auto-merging src/tools/TextImporter.java
Auto-merging src/core/TSDB.java
CONFLICT (content): Merge conflict in src/core/TSDB.java
Auto-merging src/core/IncomingDataPoints.java
Auto-merging src/core/CompactionQueue.java
Automatic merge failed; fix conflicts and then commit the result.

@pinifaran
Copy link
Author

Ok i fixed the 2 conflicts and made git commit.

@johann8384 johann8384 added this to the v2.3.0 milestone Apr 5, 2016
@johann8384 johann8384 assigned johann8384 and manolama and unassigned johann8384 Apr 5, 2016
@johann8384
Copy link
Member

Assigning to @manolama as he wanted to review this.

@pinifaran
Copy link
Author

ok, great.

@@ -57,6 +57,15 @@ public void add(final byte[] buf, final int offset, final int len) {
segments.add(new BufferSegment(buf, offset, len));
total_length += len;
}

public BufferSegment removeLastSegment() {
Copy link
Member

Choose a reason for hiding this comment

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

Jdoc and formatting.

@manolama
Copy link
Member

Thanks for the attempt!
Atomic increments are actually pretty expensive (locking, modifying plus they store 8 bytes). Therefore we don't want to penalize all TSD integers when the vast majority of metrics are going to be write-once. Instead of checking the data point type in the addPointInternal() method, we need a different API like for telnet, "putctr" and for HTTP you can add a flag that indicates they're counters. Then add a method write method that performs the increment. That way only metrics that need it can use the expensive path.

Otherwise also watch out for:

  • TSD variable naming styles (no camel case, yeah Cish)
  • Don't remove or modify public APIs, add overloads
  • JDoc all of your new functions.

Thanks!

@manolama manolama modified the milestones: v2.4.0, v2.3.0 May 1, 2016
@pinifaran
Copy link
Author

Hi Chris\Jonathan,

I've committed today the relevant fixes as you suggested:

  1. reverted back most of my previous changes (modified back the original Deferred signature, etc...).
  2. added a new command "putctr" which writes the data point value as HBase counters.
  3. relevant files for the new command:
    TSDB.java: added new method addCounter
    PutDataPointCounterRpc.java: a new file which implements writing the value as HBase counter
    RpcManager.java: registers PutDataPointCounterRpc instance for the "putctr" command.
  4. Also added comments (JDoc style) where applicable.
  5. I would be happy if you could review and send comments.

    Thanks in advance!

@johann8384 johann8384 modified the milestones: v2.3.1, v2.4.0 Jul 6, 2016
@johann8384 johann8384 modified the milestones: v2.3.1, v2.4.0 Jul 10, 2017
@Pinif
Copy link

Pinif commented May 30, 2019

I just saw 2.4.0 was released ~ 6 months ago. was this feature merged in? why is it still open?

@amariniml
Copy link

hi, this is will be merged?

@Pinif
Copy link

Pinif commented Oct 16, 2019

hi, this is will be merged?

My question exactly... I've committed this code ~ 3 years ago, i don't know why it was not merged at the time.

@johann8384 johann8384 modified the milestones: v2.5.0, 2.6.0 Oct 26, 2020
@johann8384
Copy link
Member

@Pinif Because the active maintainers got other jobs where we don't get to use OpenTSDB.

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

Successfully merging this pull request may close these issues.

5 participants